Introducing a generator configuration for GoTag customization (#485)

This patch introduces a new configuration, `go_tag`, to allow ACK
developers to customize the "Go tags" in the generated structures.
With this configuration users can easily override the default Go tags,
making adjustements like changing the tag name, adding omitempty
directives or even introducing new ones (yaml, gorm, protobuf etc).

This change helps ACK users overcome the challenge imposed by our names
utility package (`aws-controllers-k8s/pkg/names/`) which adds an
underscore suffix to field names that matches Golang keywords. Resulting
in an odd UX when consuming the generated ACK  CRDs (e.g using
`type_: <...>` instead of `type: <...>`)

We purposefully avoided directly modifying the `names` utility package
(trimming the underscores when generating the go tags) as it will break
compatibility with the already existing CRDs (yep we missed this).
Without some sort of mutating webhooks that will handle field renames,
we'll avoid going that route, and instead start using `go_tag` configs
for future CRDs/fields.

This patch will help in tweaking the generated code for
aws-controllers-k8s/eks-controller#84

Signed-off-by: Amine Hilaly <[email protected]>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

pkg/config/field.go

@@ -405,6 +405,30 @@ type FieldConfig struct {
 	// TODO(jaypipes,crtbry): Figure out if we can roll the CustomShape stuff
 	// into this type override...
 	Type *string `json:"type,omitempty"`
+	// GoTag is used to override the default Go tag injected into the fields of
+	// a generated go structure. This is useful if we want to override the json
+	// tag name or add an omitempty directive to the tag. If not specified,
+	// the default json tag is used, i.e. json:"<fieldName>,omitempty"
+	//
+	// The main reason behind introducing this feature is that, our naming utility
+	// package appends an underscore suffix to the field name if it is colliding with
+	// a Golang keyword (switch, if, else etc...). This is needed to avoid violating
+	// the Go language spec, when defining package names, variable names, etc.
+	// This functionality resulted in injecting the underscore suffix to the json tag
+	// as well, e.g. json:"type_,omitempty". Which is not ideal because it weirdens
+	// the experience for the users of the generated CRDs.
+	//
+	// One could argue that we should just modify the `names`` package to return an
+	// extra field indicating whether the field name is a Go keyword or not, or even
+	// better, return the correct go tag dirrctly. The reason why we should avoid
+	// such a change is that it would modify the already existing/generated code, which
+	// would break the compatibility for the existing CRDs. Without introducing some
+	// sort of mutating webhook to handle field name change, this is not a viable.
+	// We decided to introduce this feature to, at least, allow us to override the
+	// go tag for any new resource or fields that we generate in the future.
+	//
+	// (See https://github.com/aws-controllers-k8s/pkg/blob/main/names/names.go)
+	GoTag *string `json:"go_tag,omitempty"`
 }
 
 // GetFieldConfigs returns all FieldConfigs for a given resource as a map.

pkg/model/field.go

@@ -216,6 +216,27 @@ func (f *Field) GetFieldDocsConfig() *ackgenconfig.FieldDocsConfig {
 	return resourceConfig.Fields[f.Path]
 }
 
+// GetGoTag returns the json tag for the field. If the field has a
+// FieldConfig with a GoTag attribute, the value of GoTag is returned.
+// Otherwise, we evaluate the field type and return the appropriate
+// json tag.
+func (f *Field) GetGoTag() string {
+	// First check if the field has a GoTag attribute in the FieldConfig
+	// a.k.a generator.yaml
+	if f.FieldConfig != nil && f.FieldConfig.GoTag != nil {
+		return fmt.Sprintf("`%s`", *f.FieldConfig.GoTag)
+	}
+
+	// If the field is not required, a reference field or part of the status
+	// object, we need to inject the `omitempty`` directive into the json tag.
+	if !f.IsRequired() || f.HasReference() || f.CRD.StatusFields[f.Names.Camel] != nil {
+		return fmt.Sprintf("`json:\"%s,omitempty\"`", f.Names.CamelLower)
+
+	}
+
+	return fmt.Sprintf("`json:\"%s\"`", f.Names.CamelLower)
+}
+
 // HasReference returns true if the supplied field *path* refers to a Field
 // that contains 'ReferencesConfig' i.e. has a corresponding reference field.
 // Ex:

pkg/model/field_test.go

@@ -254,6 +254,59 @@ func TestGetReferenceFieldName(t *testing.T) {
 	}
 }
 
+func TestGetGoTag(t *testing.T) {
+	require := require.New(t)
+	assert := assert.New(t)
+	g := testutil.NewModelForServiceWithOptions(t, "eks",
+		&testutil.TestingModelOptions{
+			GeneratorConfigFile: "generator-with-gotag.yaml",
+		},
+	)
+
+	crds, err := g.GetCRDs()
+	require.Nil(err)
+
+	crd := getCRDByName("Cluster", crds)
+	require.NotNil(crd)
+
+	testCases := []struct {
+		name        string
+		field       *model.Field
+		expectedTag string
+	}{
+		{
+			name:        "not required spec field",
+			field:       crd.SpecFields["Logging"],
+			expectedTag: "`json:\"logging,omitempty\"`",
+		},
+		{
+			name:        "required spec field",
+			field:       crd.SpecFields["Name"],
+			expectedTag: "`json:\"name\"`",
+		},
+		{
+			name:        "spec field with go_tag override",
+			field:       crd.SpecFields["Version"],
+			expectedTag: "`json:\"myCustomVersionName,omitempty\"`",
+		},
+		{
+			// Status fields are not required, so they should always have omitempty directive
+			name:        "status field",
+			field:       crd.StatusFields["Endpoint"],
+			expectedTag: "`json:\"endpoint,omitempty\"`",
+		},
+		{
+			name:        "status field with go_tag override",
+			field:       crd.StatusFields["Status"],
+			expectedTag: "`json:\"clusterState,omitempty\" yaml:\"some_extra_tags\"`",
+		},
+	}
+	for _, tc := range testCases {
+		tag := tc.field.GetGoTag()
+		assert.Equal(tc.expectedTag, tag)
+	}
+}
+
 func TestReferenceFieldPath(t *testing.T) {
 	assert := assert.New(t)
 

pkg/testdata/models/apis/eks/0000-00-00/generator-with-gotag.yaml

@@ -0,0 +1,7 @@
+resources:
+  Cluster:
+    fields:
+      Version:
+        go_tag: json:"myCustomVersionName,omitempty"
+      Status:
+        go_tag: json:"clusterState,omitempty" yaml:"some_extra_tags"

templates/apis/crd.go.tpl

@@ -22,7 +22,7 @@ type {{ .CRD.Kind }}Spec struct {
 {{- if and ($field.IsRequired) (not $field.HasReference) -}}
     // +kubebuilder:validation:Required
 {{ end -}}
-    {{ $field.Names.Camel }} {{ $field.GoType }} `json:"{{ $field.Names.CamelLower }}{{- if or (not $field.IsRequired) ($field.HasReference) }},omitempty{{- end -}}"`
+    {{ $field.Names.Camel }} {{ $field.GoType }} {{ $field.GetGoTag }}
 {{- end }}
 }
 
@@ -44,7 +44,7 @@ type {{ .CRD.Kind }}Status struct {
 	{{ $field.GetDocumentation }}
 	{{- end }}
 	// +kubebuilder:validation:Optional
-	{{ $field.Names.Camel }} {{ $field.GoType }} `json:"{{ $field.Names.CamelLower }},omitempty"`
+	{{ $field.Names.Camel }} {{ $field.GoType }} {{ $field.GetGoTag }}
 {{- end }}
 }