Merge pull request #9597 from barbacbd/OCPBUGS-52203-update

OCPBUGS-52203: Find GCP KMS keys

Author: openshift-merge-bot[bot]

pkg/asset/installconfig/gcp/client.go

@@ -15,11 +15,13 @@ import (
 	dns "google.golang.org/api/dns/v1"
 	"google.golang.org/api/googleapi"
 	iam "google.golang.org/api/iam/v1"
+	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
 	"google.golang.org/api/serviceusage/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	gcpconsts "github.com/openshift/installer/pkg/constants/gcp"
+	gcptypes "github.com/openshift/installer/pkg/types/gcp"
 )
 
 //go:generate mockgen -source=./client.go -destination=./mock/gcpclient_generated.go -package=mock
@@ -54,7 +56,7 @@ type API interface {
 	ValidateServiceAccountHasPermissions(ctx context.Context, project string, permissions []string) (bool, error)
 	GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error)
 	GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error)
-	GetKeyRing(ctx context.Context, keyRingName string) (*kmspb.KeyRing, error)
+	GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error)
 }
 
 // Client makes calls to the GCP API.
@@ -585,16 +587,32 @@ func (c *Client) getKeyManagementClient(ctx context.Context) (*kms.KeyManagement
 }
 
 // GetKeyRing returns the key ring associated with the key name (if found).
-func (c *Client) GetKeyRing(ctx context.Context, keyRingName string) (*kmspb.KeyRing, error) {
+func (c *Client) GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error) {
 	kmsClient, err := c.getKeyManagementClient(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("key ring client creation failed: %w", err)
 	}
 
-	req := &kmspb.GetKeyRingRequest{Name: keyRingName}
-	resp, err := kmsClient.GetKeyRing(ctx, req)
-	if err != nil {
-		return nil, fmt.Errorf("failed to find key ring %s", keyRingName)
+	keyRingName := fmt.Sprintf("projects/%s/locations/%s/keyRings/%s", kmsKeyRef.ProjectID, kmsKeyRef.Location, kmsKeyRef.KeyRing)
+	listReq := &kmspb.ListKeyRingsRequest{
+		Parent: fmt.Sprintf("projects/%s/locations/%s", kmsKeyRef.ProjectID, kmsKeyRef.Location),
+	}
+
+	// OCPBUGS-52203:  GetKeyRingRequest{Name: keyRingName} should work but the resource name (above) is not found.
+	// The cloudkms.keyRings.list permission is required for this operation.
+	listItr := kmsClient.ListKeyRings(ctx, listReq)
+	for {
+		resp, err := listItr.Next()
+		if errors.Is(err, iterator.Done) {
+			break
+		} else if err != nil {
+			return nil, fmt.Errorf("failed to iterate through list of kms keyrings: %w", err)
+		}
+
+		re := resp
+		if re.Name == keyRingName {
+			return re, nil
+		}
 	}
-	return resp, nil
+	return nil, fmt.Errorf("failed to find kms key ring with name %s", keyRingName)
 }

pkg/asset/installconfig/gcp/mock/gcpclient_generated.go

@@ -738,7 +738,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
 	cp := ic.ControlPlane
 	validatedControlPlaneKey := false
 	if cp != nil && cp.Platform.GCP != nil && cp.Platform.GCP.EncryptionKey != nil && cp.Platform.GCP.EncryptionKey.KMSKey != nil {
-		if _, err := client.GetKeyRing(context.TODO(), cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing); err != nil {
+		if _, err := client.GetKeyRing(context.TODO(), cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey); err != nil {
 			return append(allErrs, field.Invalid(fieldPath.Child("controlPlane").Child("encryptionKey").Child("kmsKey").Child("keyRing"),
 				cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing,
 				err.Error(),
@@ -750,7 +750,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
 	validatedComputeKeys := false
 	for _, mp := range ic.Compute {
 		if mp.Platform.GCP != nil && mp.Platform.GCP.EncryptionKey != nil && mp.Platform.GCP.EncryptionKey.KMSKey != nil {
-			if _, err := client.GetKeyRing(context.TODO(), mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing); err != nil {
+			if _, err := client.GetKeyRing(context.TODO(), mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey); err != nil {
 				allErrs = append(allErrs, field.Invalid(fieldPath.Child("compute").Child("encryptionKey").Child("kmsKey").Child("keyRing"),
 					mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing,
 					err.Error(),
@@ -763,7 +763,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
 
 	defaultMp := ic.GCP.DefaultMachinePlatform
 	if defaultMp != nil && defaultMp.EncryptionKey != nil && defaultMp.EncryptionKey.KMSKey != nil {
-		if _, err := client.GetKeyRing(context.TODO(), defaultMp.EncryptionKey.KMSKey.KeyRing); err != nil {
+		if _, err := client.GetKeyRing(context.TODO(), defaultMp.EncryptionKey.KMSKey); err != nil {
 			if validatedControlPlaneKey && (validatedComputeKeys && len(allErrs) == 0) {
 				logrus.Warn("defaultMachinePool.encryptionKey.KMSKey.KeyRing is not valid, but compute and control plane key rings are valid")
 			} else {

pkg/asset/installconfig/gcp/validation.go

@@ -133,56 +133,55 @@ var (
 		)
 	}
 
+	invalidKeyRing = gcp.KMSKeyReference{
+		Name:      "invalidKeyName",
+		KeyRing:   "invalidKeyRingName",
+		Location:  "validLocation",
+		ProjectID: "validProjectID",
+	}
+
+	validKeyRing = gcp.KMSKeyReference{
+		Name:      "validKeyName",
+		KeyRing:   "validKeyRingName",
+		Location:  "validLocation",
+		ProjectID: "validProjectID",
+	}
+
 	invalidDefaultMachineKeyRing = func(ic *types.InstallConfig) {
 		ic.GCP.DefaultMachinePlatform = &gcp.MachinePool{}
 		ic.GCP.DefaultMachinePlatform.OSDisk = gcp.OSDisk{
 			EncryptionKey: &gcp.EncryptionKeyReference{
-				KMSKey: &gcp.KMSKeyReference{
-					Name:    "invalidKeyName",
-					KeyRing: "invalidKeyRingName",
-				},
+				KMSKey: &invalidKeyRing,
 			},
 		}
 	}
 
 	validCPKMSKeyRing = func(ic *types.InstallConfig) {
 		ic.ControlPlane.Platform.GCP.OSDisk = gcp.OSDisk{
 			EncryptionKey: &gcp.EncryptionKeyReference{
-				KMSKey: &gcp.KMSKeyReference{
-					Name:    "validKeyName",
-					KeyRing: "validKeyRingName",
-				},
+				KMSKey: &validKeyRing,
 			},
 		}
 	}
 	invalidateCPKMSKeyRing = func(ic *types.InstallConfig) {
 		ic.ControlPlane.Platform.GCP.OSDisk = gcp.OSDisk{
 			EncryptionKey: &gcp.EncryptionKeyReference{
-				KMSKey: &gcp.KMSKeyReference{
-					Name:    "invalidKeyName",
-					KeyRing: "invalidKeyRingName",
-				},
+				KMSKey: &invalidKeyRing,
 			},
 		}
 	}
 
 	validComputeKMSKeyRing = func(ic *types.InstallConfig) {
 		ic.Compute[0].Platform.GCP.OSDisk = gcp.OSDisk{
 			EncryptionKey: &gcp.EncryptionKeyReference{
-				KMSKey: &gcp.KMSKeyReference{
-					Name:    "validKeyName",
-					KeyRing: "validKeyRingName",
-				},
+				KMSKey: &validKeyRing,
 			},
 		}
 	}
 	invalidateComputeKMSKeyRing = func(ic *types.InstallConfig) {
 		ic.Compute[0].Platform.GCP.OSDisk = gcp.OSDisk{
 			EncryptionKey: &gcp.EncryptionKeyReference{
-				KMSKey: &gcp.KMSKeyReference{
-					Name:    "validKeyName",
-					KeyRing: "invalidKeyRingName",
-				},
+				KMSKey: &invalidKeyRing,
 			},
 		}
 	}
@@ -495,11 +494,11 @@ func TestGCPInstallConfigValidation(t *testing.T) {
 	gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, validXpnSA).Return(validXpnSA, nil).AnyTimes()
 	gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, invalidXpnSA).Return("", fmt.Errorf("controlPlane.platform.gcp.serviceAccount: Internal error\"")).AnyTimes()
 
-	validKeyRing := &kmspb.KeyRing{
+	validKeyRingRet := &kmspb.KeyRing{
 		Name: "validKeyRingName",
 	}
-	gcpClient.EXPECT().GetKeyRing(gomock.Any(), "validKeyRingName").Return(validKeyRing, nil).AnyTimes()
-	gcpClient.EXPECT().GetKeyRing(gomock.Any(), "invalidKeyRingName").Return(nil, fmt.Errorf("failed to find key ring invalidKeyRingName: data")).AnyTimes()
+	gcpClient.EXPECT().GetKeyRing(gomock.Any(), &validKeyRing).Return(validKeyRingRet, nil).AnyTimes()
+	gcpClient.EXPECT().GetKeyRing(gomock.Any(), &invalidKeyRing).Return(nil, fmt.Errorf("failed to find key ring invalidKeyRingName: data")).AnyTimes()
 
 	httpmock.Activate()
 	defer httpmock.DeactivateAndReset()