From bdfbe67ab3c2862a74619276cc985ee50ea5d6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Mon, 18 Mar 2024 17:54:54 +0300 Subject: [PATCH 1/4] Add a new ``Required` configuration option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın --- pkg/config/common.go | 1 + pkg/config/resource.go | 4 ++++ pkg/types/builder.go | 3 +++ pkg/types/field.go | 7 +++++++ 4 files changed, 15 insertions(+) diff --git a/pkg/config/common.go b/pkg/config/common.go index 651a4c88..132c6709 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -128,6 +128,7 @@ func MoveToStatus(sch *schema.Resource, fieldpaths ...string) { // useful in cases where external name contains an optional parameter that is // defaulted by the provider but we need it to exist or to fix plain buggy // schemas. +// Deprecated: Use RequiredFields API instead. func MarkAsRequired(sch *schema.Resource, fieldpaths ...string) { for _, fieldpath := range fieldpaths { if s := GetSchema(sch, fieldpath); s != nil { diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 1023fd9a..bfe6b746 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -162,6 +162,10 @@ type ExternalName struct { // management policy is including the Observe Only, different from other // (required) fields. IdentifierFields []string + + // RequiredFields are the fields that are marked as required, although + // it is not required in the TF schema. + RequiredFields []string } // References represents reference resolver configurations for the fields of a diff --git a/pkg/types/builder.go b/pkg/types/builder.go index f61e79e3..dfbe1cc3 100644 --- a/pkg/types/builder.go +++ b/pkg/types/builder.go @@ -381,6 +381,9 @@ func newTopLevelRequiredParam(path string, includeInit bool) *topLevelRequiredPa func (r *resource) addParameterField(f *Field, field *types.Var) { requiredBySchema := !f.Schema.Optional + if f.Required { + requiredBySchema = f.Required + } // Note(turkenh): We are collecting the top level required parameters that // are not identifier fields. This is for generating CEL validation rules for // those parameters and not to require them if the management policy is set diff --git a/pkg/types/field.go b/pkg/types/field.go index b4f04866..8e74de97 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -45,6 +45,7 @@ type Field struct { TransformedName string SelectorName string Identifier bool + Required 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 @@ -120,6 +121,12 @@ func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, } } + for _, required := range cfg.ExternalName.RequiredFields { + if required == snakeFieldName { + f.Required = true + } + } + var commentText string docString := getDocString(cfg, f, tfPath) if len(docString) > 0 { From f25329f346f639d1017954ecdec864adfdf87882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Tue, 19 Mar 2024 12:39:07 +0300 Subject: [PATCH 2/4] - Move the config.ExternalName.RequiredFields to config.Resource.requiredFields - Deprecate config.MarkAsRequired in favor of a new configuration function on *config.Resource that still accepts a slice to mark multiple fields as required without doing and invervention in native field schema. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın --- pkg/config/common.go | 9 ++++++++- pkg/config/resource.go | 12 ++++++++---- pkg/types/field.go | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/config/common.go b/pkg/config/common.go index 132c6709..c559bb1a 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -124,11 +124,18 @@ func MoveToStatus(sch *schema.Resource, fieldpaths ...string) { } } +// MarkAsRequired marks the given fieldpaths as required without manipulating +// the native field schema. +func (r *Resource) MarkAsRequired(fieldpaths ...string) { + r.requiredFields = append(r.requiredFields, fieldpaths...) +} + // MarkAsRequired marks the schema of the given fieldpath as required. It's most // useful in cases where external name contains an optional parameter that is // defaulted by the provider but we need it to exist or to fix plain buggy // schemas. -// Deprecated: Use RequiredFields API instead. +// Deprecated: Use Resource.MarkAsRequired instead. +// This function will be removed in future versions. func MarkAsRequired(sch *schema.Resource, fieldpaths ...string) { for _, fieldpath := range fieldpaths { if s := GetSchema(sch, fieldpath); s != nil { diff --git a/pkg/config/resource.go b/pkg/config/resource.go index bfe6b746..160ca70f 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -162,10 +162,6 @@ type ExternalName struct { // management policy is including the Observe Only, different from other // (required) fields. IdentifierFields []string - - // RequiredFields are the fields that are marked as required, although - // it is not required in the TF schema. - RequiredFields []string } // References represents reference resolver configurations for the fields of a @@ -493,6 +489,14 @@ type Resource struct { // the value of the generated Kind, for example: // "TagParameters": "ClusterTagParameters" OverrideFieldNames map[string]string + + // requiredFields are the fields that will be marked as required in the + // generated CRD schema, although they are not required in the TF schema. + requiredFields []string +} + +func (r *Resource) RequiredFields() []string { + return r.requiredFields } // ShouldUseTerraformPluginSDKClient returns whether to generate an SDKv2-based diff --git a/pkg/types/field.go b/pkg/types/field.go index 8e74de97..61e2afae 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -121,7 +121,7 @@ func NewField(g *Builder, cfg *config.Resource, r *resource, sch *schema.Schema, } } - for _, required := range cfg.ExternalName.RequiredFields { + for _, required := range cfg.RequiredFields() { if required == snakeFieldName { f.Required = true } From b73a85f49b43bdbb156f11adf364a96edabfa80d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Tue, 19 Mar 2024 12:49:33 +0300 Subject: [PATCH 3/4] Add requiredFields to ignoreUnexported for fixing 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index a8cec8d9..3bdfc553 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -128,6 +128,7 @@ func TestDefaultResource(t *testing.T) { cmpopts.IgnoreFields(ExternalName{}, "SetIdentifierArgumentFn", "GetExternalNameFn", "GetIDFn"), cmpopts.IgnoreFields(Resource{}, "useTerraformPluginSDKClient"), cmpopts.IgnoreFields(Resource{}, "useTerraformPluginFrameworkClient"), + cmpopts.IgnoreFields(Resource{}, "requiredFields"), } for name, tc := range cases { From 845dbf6b1b11cdcc329cea004fc29e2d23c5343b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Tue, 19 Mar 2024 15:26:10 +0300 Subject: [PATCH 4/4] Add doc for RequiredFields function 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 | 1 + pkg/types/builder.go | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 160ca70f..58bec39b 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -495,6 +495,7 @@ type Resource struct { requiredFields []string } +// RequiredFields returns slice of the marked as required fieldpaths. func (r *Resource) RequiredFields() []string { return r.requiredFields } diff --git a/pkg/types/builder.go b/pkg/types/builder.go index dfbe1cc3..1afb68d8 100644 --- a/pkg/types/builder.go +++ b/pkg/types/builder.go @@ -380,10 +380,7 @@ func newTopLevelRequiredParam(path string, includeInit bool) *topLevelRequiredPa } func (r *resource) addParameterField(f *Field, field *types.Var) { - requiredBySchema := !f.Schema.Optional - if f.Required { - requiredBySchema = f.Required - } + requiredBySchema := !f.Schema.Optional || f.Required // Note(turkenh): We are collecting the top level required parameters that // are not identifier fields. This is for generating CEL validation rules for // those parameters and not to require them if the management policy is set