From a7c0393822de5f35453ecf917c993d1c9bd05d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Fern=C3=A1ndez?= <7312236+fernandezcuesta@users.noreply.github.com> Date: Thu, 21 May 2026 19:48:32 +0200 Subject: [PATCH 1/2] fix: comment status without ssa options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jesús Fernández <7312236+fernandezcuesta@users.noreply.github.com> --- pkg/types/builder_test.go | 80 +++++++++++++++++++++++++++++++++++++++ pkg/types/field.go | 5 +++ 2 files changed, 85 insertions(+) diff --git a/pkg/types/builder_test.go b/pkg/types/builder_test.go index 68d5b5dd..1b6a3a4a 100644 --- a/pkg/types/builder_test.go +++ b/pkg/types/builder_test.go @@ -8,6 +8,7 @@ import ( "fmt" "go/token" "go/types" + "strings" "testing" "github.com/crossplane/crossplane-runtime/v2/pkg/test" @@ -219,6 +220,7 @@ func TestBuild(t *testing.T) { atProvider string validationRules string err error + commentChecks map[string]func(t *testing.T, comments map[string]string) } cases := map[string]struct { args @@ -637,6 +639,79 @@ func TestBuild(t *testing.T) { // +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.__namespace__) || (has(self.initProvider) && has(self.initProvider.__namespace__))",message="spec.forProvider.namespace is a required parameter"`, }, }, + "SSA_InjectedKey_Not_In_Observation_Comments": { + args: args{ + crdScope: CRDScopeCluster, + cfg: &config.Resource{ + TerraformResource: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "rule": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + "action": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + }, + }, + ServerSideApplyMergeStrategies: config.ServerSideApplyMergeStrategies{ + "rule": { + ListMergeStrategy: config.ListMergeStrategy{ + MergeStrategy: config.ListTypeMap, + ListMapKeys: config.ListMapKeys{ + InjectedKey: config.InjectedKey{ + Key: "ruleIndex", + DefaultValue: "0", + }, + }, + }, + }, + }, + }, + }, + want: want{ + forProvider: `type example.Parameters struct{Rule []example.RuleParameters "json:\"rule,omitempty\" tf:\"rule,omitempty\""}`, + atProvider: `type example.Observation struct{Rule []example.RuleObservation "json:\"rule,omitempty\" tf:\"rule,omitempty\""}`, + commentChecks: map[string]func(t *testing.T, comments map[string]string){ + "ParametersRuleHasSSAMarkers": func(t *testing.T, comments map[string]string) { + t.Helper() + v := comments["example.Parameters:Rule"] + if !strings.Contains(v, "+listType=map") { + t.Errorf("Parameters Rule comment missing +listType=map: %s", v) + } + if !strings.Contains(v, "+listMapKey=ruleIndex") { + t.Errorf("Parameters Rule comment missing +listMapKey=ruleIndex: %s", v) + } + }, + "ObservationRuleNoSSAMarkers": func(t *testing.T, comments map[string]string) { + t.Helper() + v := comments["example.Observation:Rule"] + if strings.Contains(v, "+listType=map") { + t.Errorf("Observation Rule comment must not contain +listType=map: %s", v) + } + if strings.Contains(v, "+listMapKey") { + t.Errorf("Observation Rule comment must not contain +listMapKey: %s", v) + } + }, + "ObservationInjectedKeyNoDefault": func(t *testing.T, comments map[string]string) { + t.Helper() + v := comments["example.RuleObservation:RuleIndex"] + if strings.Contains(v, "+kubebuilder:default") { + t.Errorf("Observation RuleIndex comment must not contain +kubebuilder:default: %s", v) + } + }, + }, + }, + }, } for n, tc := range cases { t.Run(n, func(t *testing.T) { @@ -659,6 +734,11 @@ func TestBuild(t *testing.T) { if diff := cmp.Diff(tc.want.validationRules, g.ValidationRules); diff != "" { t.Fatalf("Build(...): -want validationRules, +got validationRules: %s", diff) } + for checkName, checkFn := range tc.want.commentChecks { + t.Run(checkName, func(t *testing.T) { + checkFn(t, g.Comments) + }) + } }) } } diff --git a/pkg/types/field.go b/pkg/types/field.go index b492a309..a4981936 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -20,6 +20,7 @@ import ( "github.com/crossplane/upjet/v2/pkg/config" "github.com/crossplane/upjet/v2/pkg/schema/traverser" "github.com/crossplane/upjet/v2/pkg/types/comments" + "github.com/crossplane/upjet/v2/pkg/types/markers" "github.com/crossplane/upjet/v2/pkg/types/name" "github.com/crossplane/upjet/v2/pkg/types/structtag" ) @@ -429,7 +430,11 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, opt // fields under status.atProvider. So, we don't want reference comments to // be added, hence we are unsetting reference on the field comment just // before adding it as an observation field. + // Also strip SSA merge-strategy markers and injected-key defaults to avoid + // conflicts when status lists contain >1 item sharing the same default value. f.Comment.Reference = config.Reference{} + f.Comment.ServerSideApplyOptions = markers.ServerSideApplyOptions{} + f.Comment.KubebuilderOptions.Default = nil g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.Build()) } } From ab443d236179e813df1461a63d296b2cad562148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Fern=C3=A1ndez?= <7312236+fernandezcuesta@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:16:43 +0200 Subject: [PATCH 2/2] fix: only remove markers when listType: map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jesús Fernández <7312236+fernandezcuesta@users.noreply.github.com> --- pkg/types/field.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/types/field.go b/pkg/types/field.go index a4981936..6e6a0db2 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -20,7 +20,6 @@ import ( "github.com/crossplane/upjet/v2/pkg/config" "github.com/crossplane/upjet/v2/pkg/schema/traverser" "github.com/crossplane/upjet/v2/pkg/types/comments" - "github.com/crossplane/upjet/v2/pkg/types/markers" "github.com/crossplane/upjet/v2/pkg/types/name" "github.com/crossplane/upjet/v2/pkg/types/structtag" ) @@ -430,10 +429,15 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, opt // fields under status.atProvider. So, we don't want reference comments to // be added, hence we are unsetting reference on the field comment just // before adding it as an observation field. - // Also strip SSA merge-strategy markers and injected-key defaults to avoid - // conflicts when status lists contain >1 item sharing the same default value. + // Strip listType=map and its associated listMapKey markers from observation + // fields because map-type lists require an injected key with a default value, + // and that default causes conflicts with >1 items sharing a value during + // status updates. f.Comment.Reference = config.Reference{} - f.Comment.ServerSideApplyOptions = markers.ServerSideApplyOptions{} + if f.Comment.ServerSideApplyOptions.ListType != nil && *f.Comment.ServerSideApplyOptions.ListType == config.ListTypeMap { + f.Comment.ServerSideApplyOptions.ListType = nil + f.Comment.ServerSideApplyOptions.ListMapKey = nil + } f.Comment.KubebuilderOptions.Default = nil g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.Build()) }