Skip to content

Commit

Permalink
Add config.Resource.ServerSideApplyMergeStrategies to be able to conf…
Browse files Browse the repository at this point in the history
…igure

the server-side apply merge strategies for object lists & maps.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
  • Loading branch information
ulucinar committed Dec 5, 2023
1 parent ca2cfe6 commit dbea117
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 78 deletions.
23 changes: 12 additions & 11 deletions pkg/config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,18 @@ func DefaultResource(name string, terraformSchema *schema.Resource, terraformReg
}

r := &Resource{
Name: name,
TerraformResource: terraformSchema,
MetaResource: terraformRegistry,
ShortGroup: group,
Kind: kind,
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: make(map[string]*SchemaElementOption),
Name: name,
TerraformResource: terraformSchema,
MetaResource: terraformRegistry,
ShortGroup: group,
Kind: kind,
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: make(References),
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: make(SchemaElementOptions),
ServerSideApplyMergeStrategies: make(ServerSideApplyMergeStrategies),
}
for _, f := range opts {
f(r)
Expand Down
108 changes: 108 additions & 0 deletions pkg/config/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,54 @@ import (
"github.com/crossplane/upjet/pkg/registry"
)

// A ListType is a type of list.
type ListType string

// Types of lists.
const (
// ListTypeAtomic means the entire list is replaced during merge. At any
// point in time, a single manager owns the list.
ListTypeAtomic ListType = "atomic"

// ListTypeSet can be granularly merged, and different managers can own
// different elements in the list. The list can include only scalar
// elements.
ListTypeSet ListType = "set"

// ListTypeMap can be granularly merged, and different managers can own
// different elements in the list. The list can include only nested types
// (i.e. objects).
ListTypeMap ListType = "map"
)

// A MapType is a type of map.
type MapType string

// Types of maps.
const (
// MapTypeAtomic means that the map can only be entirely replaced by a
// single manager.
MapTypeAtomic MapType = "atomic"

// MapTypeGranular means that the map supports separate managers updating
// individual fields.
MapTypeGranular MapType = "granular"
)

// A StructType is a type of struct.
type StructType string

// Struct types.
const (
// StructTypeAtomic means that the struct can only be entirely replaced by a
// single manager.
StructTypeAtomic StructType = "atomic"

// StructTypeGranular means that the struct supports separate managers
// updating individual fields.
StructTypeGranular StructType = "granular"
)

// SetIdentifierArgumentsFn sets the name of the resource in Terraform attributes map,
// i.e. Main HCL file.
type SetIdentifierArgumentsFn func(base map[string]any, externalName string)
Expand Down Expand Up @@ -266,6 +314,60 @@ func setExternalTagsWithPaved(externalTags map[string]string, paved *fieldpath.P
return pavedByte, nil
}

type InjectedKey struct {
Key string
DefaultValue string
}

// ListMapKeys is the list map keys when the server-side apply merge strategy
// islistType=map.
type ListMapKeys struct {
// InjectedKey can be used to inject the specified index key
// into the generated CRD schema for the list object when
// the SSA merge strategy for the parent list is `map`.
// If a non-zero `InjectedKey` is specified, then a field of type string with
// the specified name is injected into the Terraform schema and used as
// a list map key together with any other existing keys specified in `Keys`.
InjectedKey InjectedKey
// Keys is the set of list map keys to be used while SSA merges list items.
// If InjectedKey is non-zero, then it's automatically put into Keys and
// you must not specify the InjectedKey in Keys explicitly.
Keys []string
}

// ListMergeStrategy configures the corresponding field as list
// and configures its server-side apply merge strategy.
type ListMergeStrategy struct {
// ListMapKeys is the list map keys when the SSA merge strategy is
// `listType=map`. The keys specified here must be a set of scalar Terraform
// argument names to be used as the list map keys for the object list.
ListMapKeys ListMapKeys
// MergeStrategy is the SSA merge strategy for an object list. Valid values
// are: `atomic`, `set` and `map`
MergeStrategy ListType
}

// MergeStrategy configures the server-side apply merge strategy for the
// corresponding field. One and only one of the pointer members can be set
// and the specified merge strategy configuration must match the field's
// type, e.g., you cannot set MapMergeStrategy for a field of type list.
type MergeStrategy struct {
ListMergeStrategy *ListMergeStrategy
MapMergeStrategy *MapType
StructMergeStrategy *StructType
}

// ServerSideApplyMergeStrategies configures the server-side apply merge strategy
// for the field at the specified path as the map key. The key is
// a Terraform configuration argument path such as a.b.c, without any
// index notation (i.e., array/map components do not need indices).
// It's an error to set a configuration option which does not match
// the object type at the specified path or to leave the corresponding
// configuration entry empty. For example, if the field at path a.b.c is
// a list, then ListMergeStrategy must be set and it should be the only
// configuration entry set.
type ServerSideApplyMergeStrategies map[string]MergeStrategy

// Resource is the set of information that you can override at different steps
// of the code generation pipeline.
type Resource struct {
Expand Down Expand Up @@ -338,6 +440,12 @@ type Resource struct {
// Terraform InstanceDiff is computed during reconciliation.
TerraformCustomDiff CustomDiff

// ServerSideApplyMergeStrategies configures the server-side apply merge
// strategy for the fields at the given map keys. The map key is
// a Terraform configuration argument path such as a.b.c, without any
// index notation (i.e., array/map components do not need indices).
ServerSideApplyMergeStrategies ServerSideApplyMergeStrategies

// useNoForkClient indicates that a no-fork external client should
// be generated instead of the Terraform CLI-forking client.
useNoForkClient bool
Expand Down
48 changes: 47 additions & 1 deletion pkg/types/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const (

// ref: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules
celEscapeSequence = "__%s__"
// description for an injected list map key field in the context of the
// server-side apply object list merging
descriptionInjectedKey = "This is an injected field with a default value for being able to merge items of the parent object list."
)

var (
Expand Down Expand Up @@ -67,6 +70,10 @@ func NewBuilder(pkg *types.Package) *Builder {

// Build returns parameters and observation types built out of Terraform schema.
func (g *Builder) Build(cfg *config.Resource) (Generated, error) {
if err := injectServerSideApplyListMergeKeys(cfg); err != nil {
return Generated{}, errors.Wrapf(err, "cannot inject server-side apply merge keys for resource %q", cfg.Name)
}

fp, ap, ip, err := g.buildResource(cfg.TerraformResource, cfg, nil, nil, false, cfg.Kind)
return Generated{
Types: g.genTypes,
Expand All @@ -75,7 +82,46 @@ func (g *Builder) Build(cfg *config.Resource) (Generated, error) {
InitProviderType: ip,
AtProviderType: ap,
ValidationRules: g.validationRules,
}, errors.Wrapf(err, "cannot build the Types")
}, errors.Wrapf(err, "cannot build the Types for resource %q", cfg.Name)
}

func injectServerSideApplyListMergeKeys(cfg *config.Resource) error { //nolint:gocyclo // Easier to follow the logic in a single function
for f, s := range cfg.ServerSideApplyMergeStrategies {
if s.ListMergeStrategy == nil || s.ListMergeStrategy.MergeStrategy != config.ListTypeMap {
continue
}
if s.ListMergeStrategy.ListMapKeys.InjectedKey.Key == "" && len(s.ListMergeStrategy.ListMapKeys.Keys) == 0 {
return errors.Errorf("list map keys configuration for the object list %q is empty", f)
}
if s.ListMergeStrategy.ListMapKeys.InjectedKey.Key == "" {
continue
}
sch := config.GetSchema(cfg.TerraformResource, f)
if sch == nil {
return errors.Errorf("cannot find the Terraform schema for the argument at the path %q", f)
}
if sch.Type != schema.TypeList && sch.Type != schema.TypeSet {
return errors.Errorf("fieldpath %q is not a Terraform list or set", f)
}
el, ok := sch.Elem.(*schema.Resource)
if !ok {
return errors.Errorf("fieldpath %q is a Terraform list or set but its element type is not a Terraform *schema.Resource", f)
}
for k := range el.Schema {
if k == s.ListMergeStrategy.ListMapKeys.InjectedKey.Key {
return errors.Errorf("element schema for the object list %q already contains the argument key %q", f, k)
}
}
el.Schema[s.ListMergeStrategy.ListMapKeys.InjectedKey.Key] = &schema.Schema{
Type: schema.TypeString,
Required: true,
Description: descriptionInjectedKey,
}
if s.ListMergeStrategy.ListMapKeys.InjectedKey.DefaultValue != "" {
el.Schema[s.ListMergeStrategy.ListMapKeys.InjectedKey.Key].Default = s.ListMergeStrategy.ListMapKeys.InjectedKey.DefaultValue
}
}
return nil
}

func (g *Builder) buildResource(res *schema.Resource, cfg *config.Resource, tfPath []string, xpPath []string, asBlocksMode bool, names ...string) (*types.Named, *types.Named, *types.Named, error) { //nolint:gocyclo
Expand Down
100 changes: 87 additions & 13 deletions pkg/types/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ import (
"github.com/crossplane/upjet/pkg"
"github.com/crossplane/upjet/pkg/config"
"github.com/crossplane/upjet/pkg/types/comments"
"github.com/crossplane/upjet/pkg/types/markers"
"github.com/crossplane/upjet/pkg/types/name"
)

const (
errFmtInvalidSSAConfiguration = "invalid server-side apply merge strategy configuration: Field schema for %q is of type %q and the specified configuration must only set %q"
errFmtUnsupportedSSAField = "cannot configure the server-side apply merge strategy for %q: Configuration can only be specified for lists, sets or maps"
errFmtMissingListMapKeys = "server-side apply merge strategy configuration for %q belongs to a list of type map but list map keys configuration is missing"
)

var parentheses = regexp.MustCompile(`\(([^)]+)\)`)

// Field represents a field that is built from the Terraform schema.
Expand All @@ -40,6 +45,9 @@ type Field struct {
TransformedName string
SelectorName string
Identifier bool
// Injected is set if this Field is an injected field to the Terraform
// schema as an object list map key for server-side apply merges.
Injected bool
}

// getDocString tries to extract the documentation string for the specified
Expand Down Expand Up @@ -153,32 +161,34 @@ func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema,
f.FieldType = fieldType
f.InitType = initType

if !sch.Sensitive {
AddServerSideApplyMarkers(f)
}

return f, nil
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[markers.MapType](markers.MapTypeGranular)
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[markers.ListType](markers.ListTypeSet)
f.Comment.ServerSideApplyOptions.ListType = ptr.To[config.ListType](config.ListTypeSet)
}
}
}
Expand All @@ -187,6 +197,63 @@ func AddServerSideApplyMarkers(f *Field) {
// objects with a well-known key that we could merge on?
}

func setInjectedField(fp, k string, f *Field, s config.MergeStrategy) bool {
if s.ListMergeStrategy == nil || fp != fmt.Sprintf("%s.%s", k, s.ListMergeStrategy.ListMapKeys.InjectedKey.Key) {
return false
}

if s.ListMergeStrategy.ListMapKeys.InjectedKey.DefaultValue != "" {
f.Comment.KubebuilderOptions.Default = ptr.To[string](s.ListMergeStrategy.ListMapKeys.InjectedKey.DefaultValue)
}
f.TFTag = "-" // prevent serialization into Terraform configuration
f.Injected = true
return true
}

func AddServerSideApplyMarkersFromConfig(f *Field, cfg *config.Resource) error { //nolint:gocyclo // Easier to follow the logic in a single function
// for sensitive fields, we generate secret or secret key references
if f.Schema.Sensitive {
return nil
}
fp := strings.ReplaceAll(strings.Join(f.TerraformPaths, "."), ".*.", ".")
fp = strings.TrimSuffix(fp, ".*")
for k, s := range cfg.ServerSideApplyMergeStrategies {
if setInjectedField(fp, k, f, s) || k != fp {
continue
}
switch f.Schema.Type { //nolint:exhaustive
case schema.TypeList, schema.TypeSet:
if s.ListMergeStrategy == nil || s.MapMergeStrategy != nil || s.StructMergeStrategy != nil {
return errors.Errorf(errFmtInvalidSSAConfiguration, k, "list", "ListMergeStrategy")
}
f.Comment.ServerSideApplyOptions.ListType = ptr.To[config.ListType](s.ListMergeStrategy.MergeStrategy)
if s.ListMergeStrategy.MergeStrategy != config.ListTypeMap {
continue
}
f.Comment.ServerSideApplyOptions.ListMapKey = make([]string, 0, len(s.ListMergeStrategy.ListMapKeys.Keys)+1)
f.Comment.ServerSideApplyOptions.ListMapKey = append(f.Comment.ServerSideApplyOptions.ListMapKey, s.ListMergeStrategy.ListMapKeys.Keys...)
if s.ListMergeStrategy.ListMapKeys.InjectedKey.Key != "" {
f.Comment.ServerSideApplyOptions.ListMapKey = append(f.Comment.ServerSideApplyOptions.ListMapKey, s.ListMergeStrategy.ListMapKeys.InjectedKey.Key)
}
if len(f.Comment.ServerSideApplyOptions.ListMapKey) == 0 {
return errors.Errorf(errFmtMissingListMapKeys, k)
}
case schema.TypeMap:
if s.MapMergeStrategy == nil || s.ListMergeStrategy != nil || s.StructMergeStrategy != nil {
return errors.Errorf(errFmtInvalidSSAConfiguration, k, "map", "MapMergeStrategy")
}
f.Comment.ServerSideApplyOptions.MapType = ptr.To[config.MapType](*s.MapMergeStrategy) // better to have a copy of the strategy
default:
// currently the generated APIs do not contain embedded objects, embedded
// objects are represented as lists of max size 1. However, this may
// change in the future, i.e., we may decide to generate HCL lists of max
// size 1 as embedded objects.
return errors.Errorf(errFmtUnsupportedSSAField, k)
}
}
return nil
}

// NewSensitiveField returns a constructed sensitive Field object.
func NewSensitiveField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, snakeFieldName string, tfPath, xpPath, names []string, asBlocksMode bool) (*Field, bool, error) { //nolint:gocyclo
f, err := NewField(g, cfg, r, sch, snakeFieldName, tfPath, xpPath, names, asBlocksMode)
Expand Down Expand Up @@ -267,9 +334,15 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, add
// parameter field if it's not an observation (only) field, i.e. parameter.
//
// We do this only if tf tag is not set to "-" because otherwise it won't
// be populated from the tfstate. We typically set tf tag to "-" for
// sensitive fields which were replaced with secretKeyRefs.
if f.TFTag != "-" && !addToObservation {
// be populated from the tfstate. Injected fields are included in the
// observation because an associative-list in the spec should also be
// an associative-list in the observation (status).
// We also make sure that this field has not already been added to the
// observation type via an explicit resource configuration.
// We typically set tf tag to "-" for sensitive fields which were replaced
// with secretKeyRefs, or for injected fields into the CRD schema,
// which do not exist in the Terraform schema.
if (f.TFTag != "-" || f.Injected) && !addToObservation {
r.addObservationField(f, field)
}

Expand Down Expand Up @@ -313,7 +386,8 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, add

// isInit returns true if the field should be added to initProvider.
// We don't add Identifiers, references or fields which tag is set to
// "-".
// "-" unless they are injected object list map keys for server-side apply
// merges.
//
// Identifiers as they should not be ignorable or part of init due
// the fact being created for one identifier and then updated for another
Expand All @@ -327,7 +401,7 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, add
// an earlier step, so they cannot be included as well. Plus probably they
// should also not change for Create and Update steps.
func (f *Field) isInit() bool {
return !f.Identifier && f.Reference == nil && f.TFTag != "-"
return !f.Identifier && f.Reference == nil && (f.TFTag != "-" || f.Injected)
}

func getDescription(s string) string {
Expand Down
Loading

0 comments on commit dbea117

Please sign in to comment.