Inject merge strategies into the config rather than the output, imple…

…ment support for singleton lists. Move applying SSA merge default config to provider. Use correct Computed value for list merge keys Signed-off-by: Jonathan Oddy <[email protected]>
crossplane · Mar 18, 2024 · eac88d3 · eac88d3
1 parent 2ef7077
commit eac88d3
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 33 deletions.
diff --git a/pkg/config/provider.go b/pkg/config/provider.go
@@ -351,6 +351,7 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt
 		p.Resources[name] = DefaultResource(name, terraformResource, terraformPluginFrameworkResource, providerMetadata.Resources[name], p.DefaultResourceOptions...)
 		p.Resources[name].useTerraformPluginSDKClient = isTerraformPluginSDK
 		p.Resources[name].useTerraformPluginFrameworkClient = isPluginFrameworkResource
+		p.injectServerSideApplyDefaults(p.Resources[name], terraformResource, "")
 	}
 	for i, refInjector := range p.refInjectors {
 		if err := refInjector.InjectReferences(p.Resources); err != nil {
@@ -360,6 +361,58 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt
 	return p
 }
 
+func (p *Provider) injectServerSideApplyDefaults(cfg *Resource, res *schema.Resource, basePath string) { //nolint:gocyclo // Easier to follow the logic in a single function
+
+	for k, es := range res.Schema {
+
+		var fieldPath string
+		if basePath == "" {
+			fieldPath = k
+		} else {
+			fieldPath = fmt.Sprintf("%s.%s", basePath, k)
+		}
+
+		if r, ok := es.Elem.(*schema.Resource); ok {
+			if es.Type == schema.TypeList && es.MaxItems == 1 {
+				cfg.ServerSideApplyMergeStrategies[fieldPath] = MergeStrategy{
+					ListMergeStrategy: ListMergeStrategy{
+						MergeStrategy: ListTypeMap,
+						ListMapKeys: ListMapKeys{InjectedKey: InjectedKey{
+							Key:          "ssamergekey",
+							DefaultValue: `"0"`,
+						}},
+					},
+				}
+			}
+			p.injectServerSideApplyDefaults(cfg, r, fieldPath)
+		} else {
+			switch es.Type { //nolint:exhaustive
+			case schema.TypeSet:
+				if el, ok := es.Elem.(*schema.Schema); ok {
+					switch el.Type { //nolint:exhaustive
+					case schema.TypeString, schema.TypeBool, schema.TypeInt, schema.TypeFloat:
+						cfg.ServerSideApplyMergeStrategies[fieldPath] = MergeStrategy{
+							ListMergeStrategy: ListMergeStrategy{
+								MergeStrategy: ListTypeSet,
+							},
+						}
+					}
+				}
+			case schema.TypeMap:
+				if el, ok := es.Elem.(*schema.Schema); ok {
+					switch el.Type { //nolint:exhaustive
+					case schema.TypeString, schema.TypeBool, schema.TypeInt, schema.TypeFloat:
+						// TODO(jono): I think this may be unnecessary since maps appear to default to granular?
+						cfg.ServerSideApplyMergeStrategies[fieldPath] = MergeStrategy{
+							MapMergeStrategy: MapTypeGranular,
+						}
+					}
+				}
+			}
+		}
+	}
+}
+
 // AddResourceConfigurator adds resource specific configurators.
 func (p *Provider) AddResourceConfigurator(resource string, c ResourceConfiguratorFn) { //nolint:interfacer
 	// Note(turkenh): nolint reasoning - easier to provide a function without

diff --git a/pkg/types/builder.go b/pkg/types/builder.go
@@ -115,6 +115,7 @@ func injectServerSideApplyListMergeKeys(cfg *config.Resource) error { //nolint:g
 		el.Schema[s.ListMergeStrategy.ListMapKeys.InjectedKey.Key] = &schema.Schema{
 			Type:        schema.TypeString,
 			Required:    true,
+			Computed:    sch.Computed,
 			Description: descriptionInjectedKey,
 		}
 		if s.ListMergeStrategy.ListMapKeys.InjectedKey.DefaultValue != "" {

diff --git a/pkg/types/field.go b/pkg/types/field.go
@@ -161,42 +161,9 @@ func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema,
 	f.FieldType = fieldType
 	f.InitType = initType
 
-	AddServerSideApplyMarkers(f)
 	return f, errors.Wrapf(AddServerSideApplyMarkersFromConfig(f, cfg), "cannot add the server-side apply merge strategy markers for the field")
 }
 
-// AddServerSideApplyMarkers adds server-side apply comment markers to indicate
-// that scalar maps and sets can be merged granularly, not replace atomically.
-func AddServerSideApplyMarkers(f *Field) {
-	// for sensitive fields, we generate secret or secret key references
-	if f.Schema.Sensitive {
-		return
-	}
-
-	switch f.Schema.Type { //nolint:exhaustive
-	case schema.TypeMap:
-		// A map should always have an element of type Schema.
-		if es, ok := f.Schema.Elem.(*schema.Schema); ok {
-			switch es.Type { //nolint:exhaustive
-			// We assume scalar types can be granular maps.
-			case schema.TypeString, schema.TypeBool, schema.TypeInt, schema.TypeFloat:
-				f.Comment.ServerSideApplyOptions.MapType = ptr.To[config.MapType](config.MapTypeGranular)
-			}
-		}
-	case schema.TypeSet:
-		if es, ok := f.Schema.Elem.(*schema.Schema); ok {
-			switch es.Type { //nolint:exhaustive
-			// We assume scalar types can be granular sets.
-			case schema.TypeString, schema.TypeBool, schema.TypeInt, schema.TypeFloat:
-				f.Comment.ServerSideApplyOptions.ListType = ptr.To[config.ListType](config.ListTypeSet)
-			}
-		}
-	}
-	// TODO(negz): Can we reliably add SSA markers for lists of objects? Do we
-	// have cases where we're turning a Terraform map of maps into a list of
-	// objects with a well-known key that we could merge on?
-}
-
 func setInjectedField(fp, k string, f *Field, s config.MergeStrategy) bool {
 	if fp != fmt.Sprintf("%s.%s", k, s.ListMergeStrategy.ListMapKeys.InjectedKey.Key) {
 		return false