Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config.Resource. ServerSideApplyMergeStrategies to configure the SSA Merge Strategies #308

Merged
merged 1 commit into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading