From eadba75a4f1d1b3102b1dd2799fde697b59c2c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Wed, 22 May 2024 16:14:35 +0300 Subject: [PATCH 1/2] Add a new late-init API to skip already filled field in spec.initProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın --- pkg/config/resource.go | 23 +++++++++++ pkg/pipeline/templates/terraformed.go.tmpl | 9 +++++ pkg/pipeline/terraformed.go | 3 +- pkg/resource/lateinit.go | 46 +++++++++++++++++++++- pkg/types/field.go | 8 +++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/pkg/config/resource.go b/pkg/config/resource.go index fb35aa8f..e7002629 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -220,10 +220,20 @@ type LateInitializer struct { // "block_device_mappings.ebs". IgnoredFields []string + // ConditionalIgnoredFields are the field paths to be skipped during + // late-initialization if they are filled in spec.initProvider. + ConditionalIgnoredFields []string + // ignoredCanonicalFieldPaths are the Canonical field paths to be skipped // during late-initialization. This is filled using the `IgnoredFields` // field which keeps Terraform paths by converting them to Canonical paths. ignoredCanonicalFieldPaths []string + + // conditionalIgnoredCanonicalFieldPaths are the Canonical field paths to be + // skipped during late-initialization if they are filled in spec.initProvider. + // This is filled using the `ConditionalIgnoredFields` field which keeps + // Terraform paths by converting them to Canonical paths. + conditionalIgnoredCanonicalFieldPaths []string } // GetIgnoredCanonicalFields returns the ignoredCanonicalFields @@ -239,6 +249,19 @@ func (l *LateInitializer) AddIgnoredCanonicalFields(cf string) { l.ignoredCanonicalFieldPaths = append(l.ignoredCanonicalFieldPaths, cf) } +// GetConditionalIgnoredCanonicalFields returns the conditionalIgnoredCanonicalFieldPaths +func (l *LateInitializer) GetConditionalIgnoredCanonicalFields() []string { + return l.conditionalIgnoredCanonicalFieldPaths +} + +// AddConditionalIgnoredCanonicalFields sets conditional ignored canonical fields +func (l *LateInitializer) AddConditionalIgnoredCanonicalFields(cf string) { + if l.conditionalIgnoredCanonicalFieldPaths == nil { + l.conditionalIgnoredCanonicalFieldPaths = make([]string, 0) + } + l.conditionalIgnoredCanonicalFieldPaths = append(l.conditionalIgnoredCanonicalFieldPaths, cf) +} + // GetFieldPaths returns the fieldPaths map for Sensitive func (s *Sensitive) GetFieldPaths() map[string]string { return s.fieldPaths diff --git a/pkg/pipeline/templates/terraformed.go.tmpl b/pkg/pipeline/templates/terraformed.go.tmpl index 238da214..1d400986 100644 --- a/pkg/pipeline/templates/terraformed.go.tmpl +++ b/pkg/pipeline/templates/terraformed.go.tmpl @@ -124,6 +124,15 @@ func (tr *{{ .CRD.Kind }}) LateInitialize(attrs []byte) (bool, error) { {{ range .LateInitializer.IgnoredFields -}} opts = append(opts, resource.WithNameFilter("{{ . }}")) {{ end }} + {{- if gt (len .LateInitializer.ConditionalIgnoredFields) 0 -}} + initParams, err := tr.GetInitParameters() + if err != nil { + return false, errors.Wrapf(err, "cannot get init parameters for resource '%q'", tr.GetName()) + } + {{ range .LateInitializer.ConditionalIgnoredFields -}} + opts = append(opts, resource.WithConditionalFilter("{{ . }}", initParams)) + {{ end }} + {{ end }} li := resource.NewGenericLateInitializer(opts...) return li.LateInitialize(&tr.Spec.ForProvider, params) diff --git a/pkg/pipeline/terraformed.go b/pkg/pipeline/terraformed.go index e588c00e..34cfa4d9 100644 --- a/pkg/pipeline/terraformed.go +++ b/pkg/pipeline/terraformed.go @@ -59,7 +59,8 @@ func (tg *TerraformedGenerator) Generate(cfgs []*terraformedInput, apiVersion st "Fields": cfg.Sensitive.GetFieldPaths(), } vars["LateInitializer"] = map[string]any{ - "IgnoredFields": cfg.LateInitializer.GetIgnoredCanonicalFields(), + "IgnoredFields": cfg.LateInitializer.GetIgnoredCanonicalFields(), + "ConditionalIgnoredFields": cfg.LateInitializer.GetConditionalIgnoredCanonicalFields(), } if err := trFile.Write(filePath, vars, os.ModePerm); err != nil { diff --git a/pkg/resource/lateinit.go b/pkg/resource/lateinit.go index 6d592306..d2e512f7 100644 --- a/pkg/resource/lateinit.go +++ b/pkg/resource/lateinit.go @@ -10,12 +10,14 @@ import ( "runtime/debug" "strings" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/types/name" ) const ( @@ -43,8 +45,9 @@ const ( // GenericLateInitializer performs late-initialization of a Terraformed resource. type GenericLateInitializer struct { - valueFilters []ValueFilter - nameFilters []NameFilter + valueFilters []ValueFilter + nameFilters []NameFilter + conditionalFilters []ConditionalFilter } // SetCriticalAnnotations sets the critical annotations of the resource and reports @@ -175,6 +178,35 @@ func isZeroValueOmitted(tag string) bool { return false } +// ConditionalFilter defines a late-initialization filter on CR field canonical names. +// Fields with matching cnames will not be processed during late-initialization +// if they are filled in spec.initProvider. +type ConditionalFilter func(string) bool + +// WithConditionalFilter returns a GenericLateInitializer that causes to +// skip initialization of the field with the specified canonical name +// if the field is filled in spec.initProvider. +func WithConditionalFilter(cName string, initProvider map[string]any) GenericLateInitializerOption { + return func(l *GenericLateInitializer) { + l.conditionalFilters = append(l.conditionalFilters, conditionalFilter(cName, initProvider)) + } +} + +func conditionalFilter(cName string, initProvider map[string]any) ConditionalFilter { + return func(cn string) bool { + if cName != cn { + return false + } + + paved := fieldpath.Pave(initProvider) + value, err := paved.GetValue(name.NewFromCamel(cName).Snake) + if err != nil || value == nil { + return false + } + return true + } +} + // LateInitialize Copy unset (nil) values from responseObject to crObject // Both crObject and responseObject must be pointers to structs. // Otherwise, an error will be returned. Returns `true` if at least one field has been stored @@ -230,6 +262,16 @@ func (li *GenericLateInitializer) handleStruct(parentName string, desiredObject continue } + for _, f := range li.conditionalFilters { + if f(cName) { + filtered = true + break + } + } + if filtered { + continue + } + observedStructField, _ := typeOfObservedObject.FieldByName(desiredStructField.Name) observedFieldValue := valueOfObservedObject.FieldByName(desiredStructField.Name) desiredKeepField := false diff --git a/pkg/types/field.go b/pkg/types/field.go index b85812be..398b34da 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -104,7 +104,7 @@ func getDocString(cfg *config.Resource, f *Field, tfPath []string) string { //no } // NewField returns a constructed Field object. -func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, snakeFieldName string, tfPath, xpPath, names []string, asBlocksMode bool) (*Field, error) { +func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, snakeFieldName string, tfPath, xpPath, names []string, asBlocksMode bool) (*Field, error) { //nolint:gocyclo // easy to follow f := &Field{ Schema: sch, Name: name.NewFromSnake(snakeFieldName), @@ -162,6 +162,12 @@ func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, } } + for _, ignoreField := range cfg.LateInitializer.ConditionalIgnoredFields { + if ignoreField == traverser.FieldPath(f.TerraformPaths) { + cfg.LateInitializer.AddConditionalIgnoredCanonicalFields(traverser.FieldPath(f.CanonicalPaths)) + } + } + fieldType, initType, err := g.buildSchema(f, cfg, names, traverser.FieldPath(append(tfPath, snakeFieldName)), r) if err != nil { return nil, errors.Wrapf(err, "cannot infer type from schema of field %s", f.Name.Snake) From be0cde1fe98b3f16d3f12146f21994b19923b9aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Wed, 22 May 2024 16:33:51 +0300 Subject: [PATCH 2/2] Add conditionalIgnoredCanonicalFieldPaths to IgnoreFields to fix the unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın --- pkg/config/common_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index fd643fcc..3b8252f2 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -138,7 +138,7 @@ func TestDefaultResource(t *testing.T) { // TODO(muvaf): Find a way to compare function pointers. ignoreUnexported := []cmp.Option{ cmpopts.IgnoreFields(Sensitive{}, "fieldPaths", "AdditionalConnectionDetailsFn"), - cmpopts.IgnoreFields(LateInitializer{}, "ignoredCanonicalFieldPaths"), + cmpopts.IgnoreFields(LateInitializer{}, "ignoredCanonicalFieldPaths", "conditionalIgnoredCanonicalFieldPaths"), cmpopts.IgnoreFields(ExternalName{}, "SetIdentifierArgumentFn", "GetExternalNameFn", "GetIDFn"), cmpopts.IgnoreUnexported(Resource{}), cmpopts.IgnoreUnexported(reflect.ValueOf(identityConversion).Elem().Interface()),