Skip to content

Commit

Permalink
Merge pull request #308 from ulucinar/ssa-object-lists
Browse files Browse the repository at this point in the history
Add config.Resource. ServerSideApplyMergeStrategies to configure the SSA Merge Strategies
  • Loading branch information
ulucinar authored Dec 21, 2023
2 parents 9543be9 + 0fc0a07 commit 807fb9d
Show file tree
Hide file tree
Showing 9 changed files with 323 additions and 125 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
95 changes: 50 additions & 45 deletions pkg/config/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@ func TestDefaultResource(t *testing.T) {
name: "aws_ec2_instance",
},
want: &Resource{
Name: "aws_ec2_instance",
ShortGroup: "ec2",
Kind: "Instance",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
Name: "aws_ec2_instance",
ShortGroup: "ec2",
Kind: "Instance",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
},
},
"TwoSectionsName": {
Expand All @@ -50,15 +51,16 @@ func TestDefaultResource(t *testing.T) {
name: "aws_instance",
},
want: &Resource{
Name: "aws_instance",
ShortGroup: "aws",
Kind: "Instance",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
Name: "aws_instance",
ShortGroup: "aws",
Kind: "Instance",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
},
},
"NameWithPrefixAcronym": {
Expand All @@ -67,15 +69,16 @@ func TestDefaultResource(t *testing.T) {
name: "aws_db_sql_server",
},
want: &Resource{
Name: "aws_db_sql_server",
ShortGroup: "db",
Kind: "SQLServer",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
Name: "aws_db_sql_server",
ShortGroup: "db",
Kind: "SQLServer",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
},
},
"NameWithSuffixAcronym": {
Expand All @@ -84,15 +87,16 @@ func TestDefaultResource(t *testing.T) {
name: "aws_db_server_id",
},
want: &Resource{
Name: "aws_db_server_id",
ShortGroup: "db",
Kind: "ServerID",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
Name: "aws_db_server_id",
ShortGroup: "db",
Kind: "ServerID",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
},
},
"NameWithMultipleAcronyms": {
Expand All @@ -101,15 +105,16 @@ func TestDefaultResource(t *testing.T) {
name: "aws_db_sql_server_id",
},
want: &Resource{
Name: "aws_db_sql_server_id",
ShortGroup: "db",
Kind: "SQLServerID",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
Name: "aws_db_sql_server_id",
ShortGroup: "db",
Kind: "SQLServerID",
Version: "v1alpha1",
ExternalName: NameAsIdentifier,
References: map[string]Reference{},
Sensitive: NopSensitive,
UseAsync: true,
SchemaElementOptions: SchemaElementOptions{},
ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
},
},
}
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.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
6 changes: 4 additions & 2 deletions pkg/types/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ func TestBuild(t *testing.T) {
"Invalid_Sensitive_Fields": {
args: args{
cfg: &config.Resource{
Name: "test_resource",
TerraformResource: &schema.Resource{
Schema: map[string]*schema.Schema{
"key_1": {
Expand All @@ -325,7 +326,7 @@ func TestBuild(t *testing.T) {
},
},
want: want{
err: errors.Wrapf(fmt.Errorf(`got type %q for field %q, only types "string", "*string", []string, []*string, "map[string]string" and "map[string]*string" supported as sensitive`, "*float64", "Key1"), "cannot build the Types"),
err: errors.Wrapf(fmt.Errorf(`got type %q for field %q, only types "string", "*string", []string, []*string, "map[string]string" and "map[string]*string" supported as sensitive`, "*float64", "Key1"), `cannot build the Types for resource "test_resource"`),
},
},
"References": {
Expand Down Expand Up @@ -361,6 +362,7 @@ func TestBuild(t *testing.T) {
"Invalid_Schema_Type": {
args: args{
cfg: &config.Resource{
Name: "test_resource",
TerraformResource: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Expand All @@ -372,7 +374,7 @@ func TestBuild(t *testing.T) {
},
},
want: want{
err: errors.Wrapf(errors.Wrapf(errors.Errorf("invalid schema type %s", "TypeInvalid"), "cannot infer type from schema of field %s", "name"), "cannot build the Types"),
err: errors.Wrapf(errors.Wrapf(errors.Errorf("invalid schema type %s", "TypeInvalid"), "cannot infer type from schema of field %s", "name"), `cannot build the Types for resource "test_resource"`),
},
},
"Validation_Rules_With_Keywords": {
Expand Down
Loading

0 comments on commit 807fb9d

Please sign in to comment.