Skip to content

Commit e8a1f2f

Browse files
barbacbdbarbacbd
committed
OCPBUGS-52203: Find GCP KMS keys
** The gcp validation and client in the installconfig asset was not able to find the KMS key ring. This was including keys with global and regional locations. ** The key rings should be serachable via the GetKeyRingRequest struct, but the resource name projects/*/locations/*/keyRings/* was not finding any key rings. ** The user must have the cloudkms.keyRings.list permission to list the key rings.
1 parent 84affa9 commit e8a1f2f

File tree

4 files changed

+55
-37
lines changed

4 files changed

+55
-37
lines changed

‎pkg/asset/installconfig/gcp/client.go‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ import (
1515
dns "google.golang.org/api/dns/v1"
1616
"google.golang.org/api/googleapi"
1717
iam "google.golang.org/api/iam/v1"
18+
"google.golang.org/api/iterator"
1819
"google.golang.org/api/option"
1920
"google.golang.org/api/serviceusage/v1"
2021
"k8s.io/apimachinery/pkg/util/sets"
2122

2223
gcpconsts "github.com/openshift/installer/pkg/constants/gcp"
24+
gcptypes "github.com/openshift/installer/pkg/types/gcp"
2325
)
2426

2527
//go:generate mockgen -source=./client.go -destination=./mock/gcpclient_generated.go -package=mock
@@ -54,7 +56,7 @@ type API interface {
5456
ValidateServiceAccountHasPermissions(ctx context.Context, project string, permissions []string) (bool, error)
5557
GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error)
5658
GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error)
57-
GetKeyRing(ctx context.Context, keyRingName string) (*kmspb.KeyRing, error)
59+
GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error)
5860
}
5961

6062
// Client makes calls to the GCP API.
@@ -585,16 +587,32 @@ func (c *Client) getKeyManagementClient(ctx context.Context) (*kms.KeyManagement
585587
}
586588

587589
// GetKeyRing returns the key ring associated with the key name (if found).
588-
func (c *Client) GetKeyRing(ctx context.Context, keyRingName string) (*kmspb.KeyRing, error) {
590+
func (c *Client) GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error) {
589591
kmsClient, err := c.getKeyManagementClient(ctx)
590592
if err != nil {
591593
return nil, fmt.Errorf("key ring client creation failed: %w", err)
592594
}
593595

594-
req := &kmspb.GetKeyRingRequest{Name: keyRingName}
595-
resp, err := kmsClient.GetKeyRing(ctx, req)
596-
if err != nil {
597-
return nil, fmt.Errorf("failed to find key ring %s", keyRingName)
596+
keyRingName := fmt.Sprintf("projects/%s/locations/%s/keyRings/%s", kmsKeyRef.ProjectID, kmsKeyRef.Location, kmsKeyRef.KeyRing)
597+
listReq := &kmspb.ListKeyRingsRequest{
598+
Parent: fmt.Sprintf("projects/%s/locations/%s", kmsKeyRef.ProjectID, kmsKeyRef.Location),
599+
}
600+
601+
// OCPBUGS-52203: GetKeyRingRequest{Name: keyRingName} should work but the resource name (above) is not found.
602+
// The cloudkms.keyRings.list permission is required for this operation.
603+
listItr := kmsClient.ListKeyRings(ctx, listReq)
604+
for {
605+
resp, err := listItr.Next()
606+
if errors.Is(err, iterator.Done) {
607+
break
608+
} else if err != nil {
609+
return nil, fmt.Errorf("failed to iterate through list of kms keyrings: %w", err)
610+
}
611+
612+
re := resp
613+
if re.Name == keyRingName {
614+
return re, nil
615+
}
598616
}
599-
return resp, nil
617+
return nil, fmt.Errorf("failed to find kms key ring with name %s", keyRingName)
600618
}

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

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/asset/installconfig/gcp/validation.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
738738
cp := ic.ControlPlane
739739
validatedControlPlaneKey := false
740740
if cp != nil && cp.Platform.GCP != nil && cp.Platform.GCP.EncryptionKey != nil && cp.Platform.GCP.EncryptionKey.KMSKey != nil {
741-
if _, err := client.GetKeyRing(context.TODO(), cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing); err != nil {
741+
if _, err := client.GetKeyRing(context.TODO(), cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey); err != nil {
742742
return append(allErrs, field.Invalid(fieldPath.Child("controlPlane").Child("encryptionKey").Child("kmsKey").Child("keyRing"),
743743
cp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing,
744744
err.Error(),
@@ -750,7 +750,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
750750
validatedComputeKeys := false
751751
for _, mp := range ic.Compute {
752752
if mp.Platform.GCP != nil && mp.Platform.GCP.EncryptionKey != nil && mp.Platform.GCP.EncryptionKey.KMSKey != nil {
753-
if _, err := client.GetKeyRing(context.TODO(), mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing); err != nil {
753+
if _, err := client.GetKeyRing(context.TODO(), mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey); err != nil {
754754
allErrs = append(allErrs, field.Invalid(fieldPath.Child("compute").Child("encryptionKey").Child("kmsKey").Child("keyRing"),
755755
mp.Platform.GCP.OSDisk.EncryptionKey.KMSKey.KeyRing,
756756
err.Error(),
@@ -763,7 +763,7 @@ func validatePlatformKMSKeys(client API, ic *types.InstallConfig, fieldPath *fie
763763

764764
defaultMp := ic.GCP.DefaultMachinePlatform
765765
if defaultMp != nil && defaultMp.EncryptionKey != nil && defaultMp.EncryptionKey.KMSKey != nil {
766-
if _, err := client.GetKeyRing(context.TODO(), defaultMp.EncryptionKey.KMSKey.KeyRing); err != nil {
766+
if _, err := client.GetKeyRing(context.TODO(), defaultMp.EncryptionKey.KMSKey); err != nil {
767767
if validatedControlPlaneKey && (validatedComputeKeys && len(allErrs) == 0) {
768768
logrus.Warn("defaultMachinePool.encryptionKey.KMSKey.KeyRing is not valid, but compute and control plane key rings are valid")
769769
} else {

‎pkg/asset/installconfig/gcp/validation_test.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -133,56 +133,55 @@ var (
133133
)
134134
}
135135

136+
invalidKeyRing = gcp.KMSKeyReference{
137+
Name: "invalidKeyName",
138+
KeyRing: "invalidKeyRingName",
139+
Location: "validLocation",
140+
ProjectID: "validProjectID",
141+
}
142+
143+
validKeyRing = gcp.KMSKeyReference{
144+
Name: "validKeyName",
145+
KeyRing: "validKeyRingName",
146+
Location: "validLocation",
147+
ProjectID: "validProjectID",
148+
}
149+
136150
invalidDefaultMachineKeyRing = func(ic *types.InstallConfig) {
137151
ic.GCP.DefaultMachinePlatform = &gcp.MachinePool{}
138152
ic.GCP.DefaultMachinePlatform.OSDisk = gcp.OSDisk{
139153
EncryptionKey: &gcp.EncryptionKeyReference{
140-
KMSKey: &gcp.KMSKeyReference{
141-
Name: "invalidKeyName",
142-
KeyRing: "invalidKeyRingName",
143-
},
154+
KMSKey: &invalidKeyRing,
144155
},
145156
}
146157
}
147158

148159
validCPKMSKeyRing = func(ic *types.InstallConfig) {
149160
ic.ControlPlane.Platform.GCP.OSDisk = gcp.OSDisk{
150161
EncryptionKey: &gcp.EncryptionKeyReference{
151-
KMSKey: &gcp.KMSKeyReference{
152-
Name: "validKeyName",
153-
KeyRing: "validKeyRingName",
154-
},
162+
KMSKey: &validKeyRing,
155163
},
156164
}
157165
}
158166
invalidateCPKMSKeyRing = func(ic *types.InstallConfig) {
159167
ic.ControlPlane.Platform.GCP.OSDisk = gcp.OSDisk{
160168
EncryptionKey: &gcp.EncryptionKeyReference{
161-
KMSKey: &gcp.KMSKeyReference{
162-
Name: "invalidKeyName",
163-
KeyRing: "invalidKeyRingName",
164-
},
169+
KMSKey: &invalidKeyRing,
165170
},
166171
}
167172
}
168173

169174
validComputeKMSKeyRing = func(ic *types.InstallConfig) {
170175
ic.Compute[0].Platform.GCP.OSDisk = gcp.OSDisk{
171176
EncryptionKey: &gcp.EncryptionKeyReference{
172-
KMSKey: &gcp.KMSKeyReference{
173-
Name: "validKeyName",
174-
KeyRing: "validKeyRingName",
175-
},
177+
KMSKey: &validKeyRing,
176178
},
177179
}
178180
}
179181
invalidateComputeKMSKeyRing = func(ic *types.InstallConfig) {
180182
ic.Compute[0].Platform.GCP.OSDisk = gcp.OSDisk{
181183
EncryptionKey: &gcp.EncryptionKeyReference{
182-
KMSKey: &gcp.KMSKeyReference{
183-
Name: "validKeyName",
184-
KeyRing: "invalidKeyRingName",
185-
},
184+
KMSKey: &invalidKeyRing,
186185
},
187186
}
188187
}
@@ -495,11 +494,11 @@ func TestGCPInstallConfigValidation(t *testing.T) {
495494
gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, validXpnSA).Return(validXpnSA, nil).AnyTimes()
496495
gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, invalidXpnSA).Return("", fmt.Errorf("controlPlane.platform.gcp.serviceAccount: Internal error\"")).AnyTimes()
497496

498-
validKeyRing := &kmspb.KeyRing{
497+
validKeyRingRet := &kmspb.KeyRing{
499498
Name: "validKeyRingName",
500499
}
501-
gcpClient.EXPECT().GetKeyRing(gomock.Any(), "validKeyRingName").Return(validKeyRing, nil).AnyTimes()
502-
gcpClient.EXPECT().GetKeyRing(gomock.Any(), "invalidKeyRingName").Return(nil, fmt.Errorf("failed to find key ring invalidKeyRingName: data")).AnyTimes()
500+
gcpClient.EXPECT().GetKeyRing(gomock.Any(), &validKeyRing).Return(validKeyRingRet, nil).AnyTimes()
501+
gcpClient.EXPECT().GetKeyRing(gomock.Any(), &invalidKeyRing).Return(nil, fmt.Errorf("failed to find key ring invalidKeyRingName: data")).AnyTimes()
503502

504503
httpmock.Activate()
505504
defer httpmock.DeactivateAndReset()

0 commit comments

Comments
 (0)