From a8e777002d792dfc73a074093483a98846210b64 Mon Sep 17 00:00:00 2001 From: Kumar Gaurav Sharma Date: Wed, 9 Jun 2021 22:59:19 -0700 Subject: [PATCH 1/2] Extend ack-generate to generate slice with elements of type Kubernetes Secret Related issue: https://github.com/aws-controllers-k8s/community/issues/828 With this code change, when a field of type slice is configured as secret type inside Generator config, then the generated CRD yaml allows specifying multiple k8s secret values for that field in input yaml and generated sdk code supplies these values to the resource API input field as slice type. --- pkg/generate/code/set_sdk.go | 144 +++++++++++------- pkg/generate/code/set_sdk_test.go | 43 ++++++ pkg/generate/elasticache_test.go | 9 +- .../elasticache/0000-00-00/generator.yaml | 5 +- pkg/model/field.go | 8 +- pkg/model/types.go | 9 +- 6 files changed, 154 insertions(+), 64 deletions(-) diff --git a/pkg/generate/code/set_sdk.go b/pkg/generate/code/set_sdk.go index ff838153..5eb365d5 100644 --- a/pkg/generate/code/set_sdk.go +++ b/pkg/generate/code/set_sdk.go @@ -236,17 +236,6 @@ func SetSDK( sourceAdaptedVarName += "." + f.Names.Camel sourceFieldPath := f.Names.Camel - if r.IsSecretField(memberName) { - out += setSDKForSecret( - cfg, r, - memberName, - targetVarName, - sourceAdaptedVarName, - indentLevel, - ) - continue - } - memberShapeRef, _ := inputShape.MemberRefs[memberName] memberShape := memberShapeRef.Shape @@ -339,16 +328,26 @@ func SetSDK( ) } default: - out += setSDKForScalar( - cfg, r, - memberName, - targetVarName, - inputShape.Type, - sourceFieldPath, - sourceAdaptedVarName, - memberShapeRef, - indentLevel+1, - ) + if r.IsSecretField(memberName) { + out += setSDKForSecret( + cfg, r, + memberName, + targetVarName, + sourceAdaptedVarName, + indentLevel, + ) + } else { + out += setSDKForScalar( + cfg, r, + memberName, + targetVarName, + inputShape.Type, + sourceFieldPath, + sourceAdaptedVarName, + memberShapeRef, + indentLevel+1, + ) + } } out += fmt.Sprintf( "%s}\n", indent, @@ -770,6 +769,25 @@ func setSDKForContainer( indentLevel, ) default: + if r.IsSecretField(sourceFieldPath) { + indent := strings.Repeat("\t", indentLevel) + // if ko.Spec.MasterUserPassword != nil { + out := fmt.Sprintf( + "%sif %s != nil {\n", + indent, sourceVarName, + ) + out += setSDKForSecret( + cfg, r, + "", + targetVarName, + sourceVarName, + indentLevel, + ) + // } + out += fmt.Sprintf("%s}\n", indent) + return out + } + return setSDKForScalar( cfg, r, targetFieldName, @@ -789,7 +807,6 @@ func setSDKForContainer( // // The Go code output from this function looks like this: // -// if ko.Spec.MasterUserPassword != nil { // tmpSecret, err := rm.rr.SecretValueFromReference(ctx, ko.Spec.MasterUserPassword) // if err != nil { // return nil, err @@ -797,7 +814,20 @@ func setSDKForContainer( // if tmpSecret != "" { // res.SetMasterUserPassword(tmpSecret) // } -// } +// +// or: +// +// tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f3iter) +// if err != nil { +// return nil, err +// } +// if tmpSecret != "" { +// f3elem = tmpSecret +// } +// +// The second case is used when the SecretKeyReference field +// is a slice of `[]*string` in the original AWS API Input shape. + func setSDKForSecret( cfg *ackgenconfig.Config, r *model.CRD, @@ -809,15 +839,11 @@ func setSDKForSecret( sourceVarName string, indentLevel int, ) string { + out := "" indent := strings.Repeat("\t", indentLevel) secVar := "tmpSecret" - // if ko.Spec.MasterUserPassword != nil { - out += fmt.Sprintf( - "%sif %s != nil {\n", - indent, sourceVarName, - ) // tmpSecret, err := rm.rr.SecretValueFromReference(ctx, ko.Spec.MasterUserPassword) out += fmt.Sprintf( "%s\t%s, err := rm.rr.SecretValueFromReference(ctx, %s)\n", @@ -833,13 +859,18 @@ func setSDKForSecret( // res.SetMasterUserPassword(tmpSecret) // } out += fmt.Sprintf("%s\tif tmpSecret != \"\" {\n", indent) - out += fmt.Sprintf( - "%s\t\t%s.Set%s(%s)\n", - indent, targetVarName, targetFieldName, secVar, - ) + if targetFieldName == "" { + out += fmt.Sprintf( + "%s\t\t%s = %s\n", + indent, targetVarName, secVar, + ) + } else { + out += fmt.Sprintf( + "%s\t\t%s.Set%s(%s)\n", + indent, targetVarName, targetFieldName, secVar, + ) + } out += fmt.Sprintf("%s\t}\n", indent) - // } - out += fmt.Sprintf("%s}\n", indent) return out } @@ -871,16 +902,7 @@ func setSDKForStruct( cleanMemberName := cleanMemberNames.Camel sourceAdaptedVarName := sourceVarName + "." + cleanMemberName memberFieldPath := sourceFieldPath + "." + cleanMemberName - if r.IsSecretField(memberFieldPath) { - out += setSDKForSecret( - cfg, r, - memberName, - targetVarName, - sourceAdaptedVarName, - indentLevel, - ) - continue - } + out += fmt.Sprintf( "%sif %s != nil {\n", indent, sourceAdaptedVarName, ) @@ -918,16 +940,26 @@ func setSDKForStruct( ) } default: - out += setSDKForScalar( - cfg, r, - memberName, - targetVarName, - targetShape.Type, - memberFieldPath, - sourceAdaptedVarName, - memberShapeRef, - indentLevel+1, - ) + if r.IsSecretField(memberFieldPath) { + out += setSDKForSecret( + cfg, r, + memberName, + targetVarName, + sourceAdaptedVarName, + indentLevel, + ) + } else { + out += setSDKForScalar( + cfg, r, + memberName, + targetVarName, + targetShape.Type, + memberFieldPath, + sourceAdaptedVarName, + memberShapeRef, + indentLevel+1, + ) + } } out += fmt.Sprintf( "%s}\n", indent, @@ -974,14 +1006,16 @@ func setSDKForSlice( // // f0elem.SetMyField(*f0iter) containerFieldName := "" + sourceAttributePath := sourceFieldPath if targetShape.MemberRef.Shape.Type == "structure" { containerFieldName = targetFieldName + sourceAttributePath = sourceFieldPath+"." } out += setSDKForContainer( cfg, r, containerFieldName, elemVarName, - sourceFieldPath+".", + sourceAttributePath, iterVarName, &targetShape.MemberRef, indentLevel+1, diff --git a/pkg/generate/code/set_sdk_test.go b/pkg/generate/code/set_sdk_test.go index 1c2fa139..862564af 100644 --- a/pkg/generate/code/set_sdk_test.go +++ b/pkg/generate/code/set_sdk_test.go @@ -1109,6 +1109,49 @@ func TestSetSDK_Elasticache_ReplicationGroup_Update_Override_Values(t *testing.T ) } +func TestSetSDK_Elasticache_User_Create_Override_Values(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + g := testutil.NewGeneratorForService(t, "elasticache") + + crd := testutil.GetCRDByName(t, g, "User") + require.NotNil(crd) + + expected := ` + if r.ko.Spec.AccessString != nil { + res.SetAccessString(*r.ko.Spec.AccessString) + } + if r.ko.Spec.NoPasswordRequired != nil { + res.SetNoPasswordRequired(*r.ko.Spec.NoPasswordRequired) + } + if r.ko.Spec.Passwords != nil { + f3 := []*string{} + for _, f3iter := range r.ko.Spec.Passwords { + var f3elem string + if f3iter != nil { + tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f3iter) + if err != nil { + return nil, err + } + if tmpSecret != "" { + f3elem = tmpSecret + } + } + f3 = append(f3, &f3elem) + } + res.SetPasswords(f3) + } + if r.ko.Spec.UserID != nil { + res.SetUserId(*r.ko.Spec.UserID) + } +` + assert.Equal( + expected, + code.SetSDK(crd.Config(), crd, model.OpTypeUpdate, "r.ko", "res", 1), + ) +} + func TestSetSDK_RDS_DBInstance_Create(t *testing.T) { assert := assert.New(t) require := require.New(t) diff --git a/pkg/generate/elasticache_test.go b/pkg/generate/elasticache_test.go index 59c66ab8..7245c386 100644 --- a/pkg/generate/elasticache_test.go +++ b/pkg/generate/elasticache_test.go @@ -274,6 +274,13 @@ func TestElasticache_ValidateAuthTokenIsSecret(t *testing.T) { assert := assert.New(t) assert.Equal("*ackv1alpha1.SecretKeyReference", crd.SpecFields["AuthToken"].GoType) - assert.Equal("ackv1alpha1.SecretKeyReference", crd.SpecFields["AuthToken"].GoTypeElem) + assert.Equal("SecretKeyReference", crd.SpecFields["AuthToken"].GoTypeElem) assert.Equal("*ackv1alpha1.SecretKeyReference", crd.SpecFields["AuthToken"].GoTypeWithPkgName) + + crd = getCRDByName("User", crds) + require.NotNil(crd) + + assert.Equal("[]*ackv1alpha1.SecretKeyReference", crd.SpecFields["Passwords"].GoType) + assert.Equal("SecretKeyReference", crd.SpecFields["Passwords"].GoTypeElem) + assert.Equal("[]*ackv1alpha1.SecretKeyReference", crd.SpecFields["Passwords"].GoTypeWithPkgName) } \ No newline at end of file diff --git a/pkg/generate/testdata/models/apis/elasticache/0000-00-00/generator.yaml b/pkg/generate/testdata/models/apis/elasticache/0000-00-00/generator.yaml index 5aeb9063..5d5c79cc 100644 --- a/pkg/generate/testdata/models/apis/elasticache/0000-00-00/generator.yaml +++ b/pkg/generate/testdata/models/apis/elasticache/0000-00-00/generator.yaml @@ -18,6 +18,10 @@ resources: from: operation: DescribeEvents path: Events + User: + fields: + Passwords: + is_secret: true ReplicationGroup: update_conditions_custom_method_name: CustomUpdateConditions exceptions: @@ -126,7 +130,6 @@ ignore: - GlobalReplicationGroup - CacheCluster - CacheSecurityGroup - - User - UserGroup field_paths: - DescribeSnapshotsInput.CacheClusterId diff --git a/pkg/model/field.go b/pkg/model/field.go index d1117d17..b2ff75c1 100644 --- a/pkg/model/field.go +++ b/pkg/model/field.go @@ -95,12 +95,8 @@ func NewField( shape = shapeRef.Shape } - if cfg != nil && cfg.IsSecret { - gt = "*ackv1alpha1.SecretKeyReference" - gte = "ackv1alpha1.SecretKeyReference" - gtwp = "*ackv1alpha1.SecretKeyReference" - } else if shape != nil { - gte, gt, gtwp = cleanGoType(crd.sdkAPI, crd.cfg, shape) + if shape != nil { + gte, gt, gtwp = cleanGoType(crd.sdkAPI, crd.cfg, shape, cfg) } else { gte = "string" gt = "*string" diff --git a/pkg/model/types.go b/pkg/model/types.go index ff25ea61..a23ec8f3 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -29,6 +29,7 @@ func cleanGoType( api *SDKAPI, cfg *ackgenconfig.Config, shape *awssdkmodel.Shape, + fieldCfg *ackgenconfig.FieldConfig, ) (string, string, string) { // There are shapes that are called things like DBProxyStatus that are // fields in a DBProxy CRD... we need to ensure the type names don't @@ -48,7 +49,7 @@ func cleanGoType( } else if shape.Type == "list" { // If it's a list type, where the element is a structure, we need to // set the GoType to the cleaned-up Camel-cased name - mgte, mgt, _ := cleanGoType(api, cfg, shape.MemberRef.Shape) + mgte, mgt, mgtwp := cleanGoType(api, cfg, shape.MemberRef.Shape, fieldCfg) cleanNames := names.New(mgte) gte = cleanNames.Camel if api.HasConflictingTypeName(mgte, cfg) { @@ -56,12 +57,18 @@ func cleanGoType( } gt = "[]" + mgt + gtwp = "[]" + mgtwp } else if shape.Type == "timestamp" { // time.Time needs to be converted to apimachinery/metav1.Time // otherwise there is no DeepCopy support gtwp = "*metav1.Time" gte = "metav1.Time" gt = "*metav1.Time" + } else if fieldCfg != nil && fieldCfg.IsSecret { + gt = "*ackv1alpha1.SecretKeyReference" + gte = "SecretKeyReference" + gtwp = "*ackv1alpha1.SecretKeyReference" + return gte, gt, gtwp } // Replace the type part of the full type-with-package-name with the From ec89d92e93db610e5e65582222257bf8155d15b8 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Thu, 10 Jun 2021 20:26:11 -0400 Subject: [PATCH 2/2] correcting spaces/TABs in comment... --- pkg/generate/code/set_sdk.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/generate/code/set_sdk.go b/pkg/generate/code/set_sdk.go index 5eb365d5..7e73f8c2 100644 --- a/pkg/generate/code/set_sdk.go +++ b/pkg/generate/code/set_sdk.go @@ -817,9 +817,9 @@ func setSDKForContainer( // // or: // -// tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f3iter) -// if err != nil { -// return nil, err +// tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f3iter) +// if err != nil { +// return nil, err // } // if tmpSecret != "" { // f3elem = tmpSecret