Skip to content

Commit c8f47cc

Browse files
fix: support list and map element types in compareSlice for delta code generation (#675)
## Fix: Support list/map element types in compareSlice for delta code generation ### Problem Code generation fails when a resource has a field that is a list-of-lists or list-of-maps. The `compareSlice` function in `compare.go` only handled `string`, `structure`, and `union` element types, returning an error for `list` and `map` types: ``` field "Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations": unsupported element type in compareSlice: list ``` This blocked generating the QuickSight `DataSet` resource, where `TagRuleConfigurations` is `[][]*string`. ### Fix Added `list` and `map` as supported element types in `compareSlice`, using `equality.Semantic.Equalities.DeepEqual` for comparison — the same approach already used for `structure` and `union` types. ### Testing - Added `TestCompareResource_QuickSight_DataSet` with a minimal QuickSight Smithy model containing just the DataSet CRUD shapes - Test validates the exact field (`TagRuleConfigurations`) that triggered the original error - All existing compare tests continue to pass ### Changes - `pkg/generate/code/compare.go` — added `list`, `map` case to `compareSlice` - `pkg/generate/code/compare_test.go` — added QuickSight DataSet test - `pkg/testdata/` — added QuickSight test model and generator config By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 5d07f46 commit c8f47cc

4 files changed

Lines changed: 6345 additions & 0 deletions

File tree

pkg/generate/code/compare.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,13 @@ func compareSlice(
525525
"%sif !equality.Semantic.Equalities.DeepEqual(%s, %s) {\n",
526526
indent, firstResVarName, secondResVarName,
527527
)
528+
case "list", "map":
529+
// For nested collection types (e.g. [][]*string or []map[string]*string),
530+
// use DeepEqual since there's no simple element-wise comparison available.
531+
out += fmt.Sprintf(
532+
"%sif !equality.Semantic.Equalities.DeepEqual(%s, %s) {\n",
533+
indent, firstResVarName, secondResVarName,
534+
)
528535
default:
529536
return "", fmt.Errorf("field %q: unsupported element type in compareSlice: %s", fieldPath, elemType)
530537
}

pkg/generate/code/compare_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,3 +668,89 @@ func TestCompareResource_IAM_Role_IAMPolicy(t *testing.T) {
668668
require.NoError(err)
669669
assert.Equal(expected, got)
670670
}
671+
672+
// TestCompareResource_QuickSight_DataSet tests that the delta code generation
673+
// correctly handles the QuickSight DataSet resource, specifically the
674+
// RowLevelPermissionTagConfiguration.TagRuleConfigurations field which is a
675+
// list-of-lists-of-strings ([][]*string). This was the original field that
676+
// triggered the "unsupported element type in compareSlice: list" error.
677+
func TestCompareResource_QuickSight_DataSet(t *testing.T) {
678+
assert := assert.New(t)
679+
require := require.New(t)
680+
681+
g := testutil.NewModelForService(t, "quicksight")
682+
683+
crd := testutil.GetCRDByName(t, g, "DataSet")
684+
require.NotNil(crd)
685+
686+
// RowLevelPermissionTagConfiguration.TagRuleConfigurations is [][]*string
687+
// (a list where each element is itself a list of strings). The generated
688+
// comparison code should use equality.Semantic.Equalities.DeepEqual.
689+
expected := `
690+
if ackcompare.HasNilDifference(a.ko.Spec.AWSAccountID, b.ko.Spec.AWSAccountID) {
691+
delta.Add("Spec.AWSAccountID", a.ko.Spec.AWSAccountID, b.ko.Spec.AWSAccountID)
692+
} else if a.ko.Spec.AWSAccountID != nil && b.ko.Spec.AWSAccountID != nil {
693+
if *a.ko.Spec.AWSAccountID != *b.ko.Spec.AWSAccountID {
694+
delta.Add("Spec.AWSAccountID", a.ko.Spec.AWSAccountID, b.ko.Spec.AWSAccountID)
695+
}
696+
}
697+
if ackcompare.HasNilDifference(a.ko.Spec.DataSetID, b.ko.Spec.DataSetID) {
698+
delta.Add("Spec.DataSetID", a.ko.Spec.DataSetID, b.ko.Spec.DataSetID)
699+
} else if a.ko.Spec.DataSetID != nil && b.ko.Spec.DataSetID != nil {
700+
if *a.ko.Spec.DataSetID != *b.ko.Spec.DataSetID {
701+
delta.Add("Spec.DataSetID", a.ko.Spec.DataSetID, b.ko.Spec.DataSetID)
702+
}
703+
}
704+
if ackcompare.HasNilDifference(a.ko.Spec.ImportMode, b.ko.Spec.ImportMode) {
705+
delta.Add("Spec.ImportMode", a.ko.Spec.ImportMode, b.ko.Spec.ImportMode)
706+
} else if a.ko.Spec.ImportMode != nil && b.ko.Spec.ImportMode != nil {
707+
if *a.ko.Spec.ImportMode != *b.ko.Spec.ImportMode {
708+
delta.Add("Spec.ImportMode", a.ko.Spec.ImportMode, b.ko.Spec.ImportMode)
709+
}
710+
}
711+
if ackcompare.HasNilDifference(a.ko.Spec.Name, b.ko.Spec.Name) {
712+
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
713+
} else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil {
714+
if *a.ko.Spec.Name != *b.ko.Spec.Name {
715+
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
716+
}
717+
}
718+
if ackcompare.HasNilDifference(a.ko.Spec.RowLevelPermissionTagConfiguration, b.ko.Spec.RowLevelPermissionTagConfiguration) {
719+
delta.Add("Spec.RowLevelPermissionTagConfiguration", a.ko.Spec.RowLevelPermissionTagConfiguration, b.ko.Spec.RowLevelPermissionTagConfiguration)
720+
} else if a.ko.Spec.RowLevelPermissionTagConfiguration != nil && b.ko.Spec.RowLevelPermissionTagConfiguration != nil {
721+
if ackcompare.HasNilDifference(a.ko.Spec.RowLevelPermissionTagConfiguration.Status, b.ko.Spec.RowLevelPermissionTagConfiguration.Status) {
722+
delta.Add("Spec.RowLevelPermissionTagConfiguration.Status", a.ko.Spec.RowLevelPermissionTagConfiguration.Status, b.ko.Spec.RowLevelPermissionTagConfiguration.Status)
723+
} else if a.ko.Spec.RowLevelPermissionTagConfiguration.Status != nil && b.ko.Spec.RowLevelPermissionTagConfiguration.Status != nil {
724+
if *a.ko.Spec.RowLevelPermissionTagConfiguration.Status != *b.ko.Spec.RowLevelPermissionTagConfiguration.Status {
725+
delta.Add("Spec.RowLevelPermissionTagConfiguration.Status", a.ko.Spec.RowLevelPermissionTagConfiguration.Status, b.ko.Spec.RowLevelPermissionTagConfiguration.Status)
726+
}
727+
}
728+
if len(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations) != len(b.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations) {
729+
delta.Add("Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations", a.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations)
730+
} else if len(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations) > 0 {
731+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations) {
732+
delta.Add("Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations", a.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRuleConfigurations)
733+
}
734+
}
735+
if len(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRules) != len(b.ko.Spec.RowLevelPermissionTagConfiguration.TagRules) {
736+
delta.Add("Spec.RowLevelPermissionTagConfiguration.TagRules", a.ko.Spec.RowLevelPermissionTagConfiguration.TagRules, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRules)
737+
} else if len(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRules) > 0 {
738+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.RowLevelPermissionTagConfiguration.TagRules, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRules) {
739+
delta.Add("Spec.RowLevelPermissionTagConfiguration.TagRules", a.ko.Spec.RowLevelPermissionTagConfiguration.TagRules, b.ko.Spec.RowLevelPermissionTagConfiguration.TagRules)
740+
}
741+
}
742+
}
743+
if len(a.ko.Spec.Tags) != len(b.ko.Spec.Tags) {
744+
delta.Add("Spec.Tags", a.ko.Spec.Tags, b.ko.Spec.Tags)
745+
} else if len(a.ko.Spec.Tags) > 0 {
746+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.Tags, b.ko.Spec.Tags) {
747+
delta.Add("Spec.Tags", a.ko.Spec.Tags, b.ko.Spec.Tags)
748+
}
749+
}
750+
`
751+
got, err := code.CompareResource(
752+
crd.Config(), crd, "delta", "a.ko", "b.ko", 1,
753+
)
754+
require.NoError(err)
755+
assert.Equal(expected, got)
756+
}

0 commit comments

Comments
 (0)