Create separate documentation.yaml file (#447)

Closes aws-controllers-k8s/community#1223
Expands upon #394

Description of changes:
This pull request moves the custom GoDoc overrides from inside the `generator.yaml` file to a new, separate `documentation.yaml` file. This `documentation.yaml` file currently supports modify field GoDocs (but could be extended to modify resource GoDocs as well) in the following ways:
- Prepend: Add a comment before the existing shape ref GoDoc
- Append: Add a comment after the existing shape ref GoDoc
- Override: Completely replace the existing shape ref GoDoc

The current RDS documentation modifications are now modeled in the `documentation.yaml` like so:
```yaml
resources:
  DBClusterParameterGroup:
    fields:
      Parameters: 
        override: |
          DEPRECATED - do not use.  Prefer ParameterOverrides instead.
      ParameterOverrides: 
        append: |
          These are ONLY user-defined parameter overrides for the
          DB cluster parameter group. This does not contain default or system
          parameters.
  DBParameterGroup:
    fields:
      ParameterOverrides: 
        append: |
          These are ONLY user-defined parameter overrides for the DB parameter
          group. This does not contain default or system parameters.
```

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

Author: RedbackThomson

cmd/ack-generate/command/common.go

@@ -68,8 +68,13 @@ func loadModel(svcAlias string, apiVersion string, apiGroup string, defaultCfg a
 		sdkAPI.APIGroupSuffix = apiGroup
 	}
 
+	docCfg, err := ackgenconfig.NewDocumentationConfig(optDocumentationConfigPath)
+	if err != nil {
+		return nil, err
+	}
+
 	m, err := ackmodel.New(
-		sdkAPI, svcAlias, apiVersion, cfg,
+		sdkAPI, svcAlias, apiVersion, cfg, docCfg,
 	)
 	if err != nil {
 		return nil, err

cmd/ack-generate/command/root.go

@@ -30,21 +30,22 @@ A tool to generate AWS service controller code`
 )
 
 var (
-	defaultCacheDir        string
-	optCacheDir            string
-	optRefreshCache        bool
-	optAWSSDKGoVersion     string
-	defaultTemplateDirs    []string
-	optTemplateDirs        []string
-	defaultServicesDir     string
-	optServicesDir         string
-	optDryRun              bool
-	sdkDir                 string
-	optGeneratorConfigPath string
-	optMetadataConfigPath  string
-	optOutputPath          string
-	optServiceAccountName  string
-	optImageRepository     string
+	defaultCacheDir            string
+	optCacheDir                string
+	optRefreshCache            bool
+	optAWSSDKGoVersion         string
+	defaultTemplateDirs        []string
+	optTemplateDirs            []string
+	defaultServicesDir         string
+	optServicesDir             string
+	optDryRun                  bool
+	sdkDir                     string
+	optGeneratorConfigPath     string
+	optMetadataConfigPath      string
+	optDocumentationConfigPath string
+	optOutputPath              string
+	optServiceAccountName      string
+	optImageRepository         string
 )
 
 var rootCmd = &cobra.Command{
@@ -117,6 +118,9 @@ func init() {
 	rootCmd.PersistentFlags().StringVar(
 		&optMetadataConfigPath, "metadata-config-path", "", "Path to file containing service metadata to use",
 	)
+	rootCmd.PersistentFlags().StringVar(
+		&optDocumentationConfigPath, "documentation-config-path", "", "Path to file containing additions for the API documentation fields",
+	)
 	rootCmd.PersistentFlags().StringVarP(
 		&optOutputPath, "output", "o", "", "Path to directory to output generated files (if generating crossplane providers, this should be the root of the aws-crossplane directory)",
 	)

pkg/config/documentation.go

@@ -0,0 +1,51 @@
+package config
+
+import (
+	"io/ioutil"
+
+	"github.com/ghodss/yaml"
+)
+
+// DocumentationConfig represents the configuration of the documentation file,
+// used to override or append documentation to any of the resource fields
+type DocumentationConfig struct {
+	Resources map[string]*ResourceDocsConfig `json:"resources,omitempty"`
+}
+
+// ResourceDocsConfig represents the configuration for the documentation
+// overrides of a single resource
+type ResourceDocsConfig struct {
+	Fields map[string]*FieldDocsConfig `json:"fields,omitempty"`
+}
+
+// FieldDocsConfig represents the configuration for the documentation overrides
+// of a single field
+type FieldDocsConfig struct {
+	// Append specifies a string that will be added to the end of the existing
+	// GoDoc comment for the field
+	Append *string `json:"append,omitempty"`
+	// Prepend specifies a string that will be added before the existing
+	// GoDoc comment for the field
+	Prepend *string `json:"prepend,omitempty"`
+	// Override will entirely replace the GoDoc comment for the field
+	Override *string `json:"override,omitempty"`
+}
+
+// NewDocumentationConfig returns a new DocumentationConfig object given a
+// supplied path to a config file
+func NewDocumentationConfig(
+	configPath string,
+) (DocumentationConfig, error) {
+	if configPath == "" {
+		return DocumentationConfig{}, nil
+	}
+	content, err := ioutil.ReadFile(configPath)
+	if err != nil {
+		return DocumentationConfig{}, err
+	}
+	gc := DocumentationConfig{}
+	if err = yaml.Unmarshal(content, &gc); err != nil {
+		return DocumentationConfig{}, err
+	}
+	return gc, nil
+}

pkg/config/field.go

@@ -405,11 +405,6 @@ type FieldConfig struct {
 	// TODO(jaypipes,crtbry): Figure out if we can roll the CustomShape stuff
 	// into this type override...
 	Type *string `json:"type,omitempty"`
-	// Documentation is a string that is added *in addition to* any existing
-	// field documentation derived from the field's doc-2.json contents. For
-	// custom fields, this allows you to add custom documentation for the
-	// field.
-	Documentation *string `json:"documentation,omitempty"`
 }
 
 // GetFieldConfigs returns all FieldConfigs for a given resource as a map.

pkg/metadata/api_info.go

@@ -29,6 +29,8 @@ type APIInfo struct {
 	AWSSDKVersion string
 	// Full path of the generator config file.
 	GeneratorConfigPath string
+	// Full path of the documentation config file.
+	DocumentationConfigPath string
 	// The API version.
 	APIVersion string
 }

pkg/model/crd.go

@@ -64,6 +64,7 @@ func (ops Ops) IterOps() []*awssdkmodel.Operation {
 type CRD struct {
 	sdkAPI *SDKAPI
 	cfg    *ackgenconfig.Config
+	docCfg *ackgenconfig.DocumentationConfig
 	Names  names.Names
 	Kind   string
 	Plural string
@@ -785,6 +786,7 @@ func (r *CRD) SortedFieldNames() []string {
 func NewCRD(
 	sdkAPI *SDKAPI,
 	cfg *ackgenconfig.Config,
+	docCfg *ackgenconfig.DocumentationConfig,
 	crdNames names.Names,
 	ops Ops,
 ) *CRD {
@@ -794,6 +796,7 @@ func NewCRD(
 	return &CRD{
 		sdkAPI:                   sdkAPI,
 		cfg:                      cfg,
+		docCfg:                   docCfg,
 		Names:                    crdNames,
 		Kind:                     kind,
 		Plural:                   plural,

pkg/model/field.go

@@ -65,19 +65,20 @@ type Field struct {
 }
 
 // GetDocumentation returns a string containing the field's
-// description/docstring. If the field has a ShapeRef that has non-empty
-// Documentation AND the field has a Documentation configuration option, then
-// the docstring contained in the Documentation configuration option will be
-// appended to ShapeRef's docstring following 2 line breaks with Go comment
-// line beginnings.
+// description/docstring. The ShapeRef from the AWS SDK is used as the default
+// value for the documentation of the field. If a documentation override has
+// been provided (in the `documentation.yaml`) file, then it will either be
+// prependend or appended, or entirely overridden.
 //
-// In other words, if there is a field with a ShapeRef that has a Documentation string containing:
+// For example, if there is a field with a ShapeRef that has a Documentation
+// string containing:
 //
 // "// This field contains the identifier for the cluster
 //
-//	// running the cache services"
+//	running the cache services"
 //
-// and the field has a FieldConfig.Documentation string containing:
+// and the field has a documentation config specifies the following should be
+// appended:
 //
 // "please note that this field is updated on the service
 //
@@ -88,36 +89,76 @@ type Field struct {
 // "// This field contains the identifier for the cluster
 //
 //	// running the cache services
-//	//
 //	// please note that this field is updated on the service
 //	// side"
 func (f *Field) GetDocumentation() string {
+	cfg := f.GetFieldDocsConfig()
+
 	hasShapeDoc := false
-	var sb strings.Builder
+	var prepend strings.Builder
+	var out strings.Builder
+
 	if f.ShapeRef != nil {
 		if f.ShapeRef.Documentation != "" {
 			hasShapeDoc = true
-			sb.WriteString(f.ShapeRef.Documentation)
+			out.WriteString(f.ShapeRef.Documentation)
 		}
 	}
-	if f.FieldConfig != nil {
-		if f.FieldConfig.Documentation != nil {
-			if hasShapeDoc {
-				sb.WriteString("\n//\n")
-			}
-			// Strip any leading comment slashes from the config option
-			// docstring since we'll be automatically adding the Go comment
-			// slashes to the beginning of each new line
-			cfgDoc := *f.FieldConfig.Documentation
-			lines := strings.Split(cfgDoc, "\n")
-			numLines := len(lines)
-			for x, line := range lines {
-				sb.WriteString("// ")
-				sb.WriteString(strings.TrimLeft(line, "/ "))
-				if x < (numLines - 1) {
-					sb.WriteString("\n")
-				}
-			}
+	if cfg == nil {
+		return out.String()
+	}
+
+	// Ensure configuration has exclusive options
+	if cfg.Override != nil && (cfg.Append != nil || cfg.Prepend != nil) {
+		panic("Documentation cannot contain override and prepend/append. Field: " + f.CRD.Kind)
+	}
+
+	if cfg.Override != nil {
+		return f.formatUserProvidedDocstring(*cfg.Override)
+	}
+
+	if cfg.Append != nil {
+		if hasShapeDoc {
+			out.WriteString("\n//\n")
+		}
+		out.WriteString(f.formatUserProvidedDocstring(*cfg.Append))
+	}
+
+	if cfg.Prepend != nil {
+		prepend.WriteString(f.formatUserProvidedDocstring(*cfg.Prepend))
+		if hasShapeDoc {
+			prepend.WriteString("\n//\n")
+		}
+	}
+
+	return prepend.String() + out.String()
+}
+
+// formatUserProvidedDocstring sanitises a doc string provided by the user so
+// that it fits the format of the existing AWS SDK GoDocs. This method will
+// split it by lines, strip it of leading whitespace and slashes and prepend
+// `//` to each line.
+func (f *Field) formatUserProvidedDocstring(in string) string {
+	var sb strings.Builder
+	// Strip any leading comment slashes from the config option
+	// docstring since we'll be automatically adding the Go comment
+	// slashes to the beginning of each new line
+	lines := strings.Split(in, "\n")
+
+	// Traverse in reverse order until, deleting empty lines
+	for i := len(lines) - 1; i >= 0; i-- {
+		if strings.TrimSpace(lines[i]) != "" {
+			break
+		}
+		lines = lines[:i]
+	}
+
+	numLines := len(lines)
+	for idx, line := range lines {
+		sb.WriteString("// ")
+		sb.WriteString(strings.TrimLeft(line, " \\\t/"))
+		if idx < (numLines - 1) {
+			sb.WriteString("\n")
 		}
 	}
 	return sb.String()
@@ -164,6 +205,17 @@ func (f *Field) GetSetterConfig(opType OpType) *ackgenconfig.SetFieldConfig {
 	return nil
 }
 
+// GetFieldDocsConfig returns the field documentation configuration for the
+// current field if it exists, otherwise it returns nil.
+func (f *Field) GetFieldDocsConfig() *ackgenconfig.FieldDocsConfig {
+	resourceConfig, exists := f.CRD.docCfg.Resources[f.CRD.Names.Camel]
+	if !exists {
+		return nil
+	}
+
+	return resourceConfig.Fields[f.Path]
+}
+
 // 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

@@ -2,6 +2,7 @@ package model_test
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/aws-controllers-k8s/pkg/names"
@@ -17,44 +18,57 @@ import (
 func TestFieldDocumentation(t *testing.T) {
 	require := require.New(t)
 
-	g := testutil.NewModelForServiceWithOptions(t, "ec2",
+	g := testutil.NewModelForServiceWithOptions(t, "eks",
 		&testutil.TestingModelOptions{
-			GeneratorConfigFile: "generator-with-doc-overrides.yaml",
+			GeneratorConfigFile:     "generator.yaml",
+			DocumentationConfigFile: "documentation.yaml",
 		},
 	)
 
 	crds, err := g.GetCRDs()
 	require.Nil(err)
 
-	crd := getCRDByName("LaunchTemplate", crds)
+	crd := getCRDByName("Cluster", crds)
 	require.NotNil(crd)
 
 	specFields := crd.SpecFields
 
-	// We have not altered the docstring for LaunchTemplateData from the
+	// We have not altered the docstring for Version from the
 	// docstring that comes in the doc-2.json file...
-	ltdField := specFields["LaunchTemplateData"]
+	ltdField := specFields["Version"]
 	require.NotNil(ltdField)
 	require.NotNil(ltdField.ShapeRef)
 
 	require.Equal(
-		"// The information for the launch template.",
+		"// The desired Kubernetes version for your cluster. If you don't specify a value\n// here, the latest version available in Amazon EKS is used.",
 		ltdField.GetDocumentation(),
 	)
 
-	// We have added an additional docstring for
-	// LaunchTemplateData.HibernationOptions.Configured to the docstring that
-	// comes in the doc-2.json file...
-	hoField := ltdField.MemberFields["HibernationOptions"]
-	require.NotNil(hoField)
-	require.NotNil(hoField.ShapeRef)
-	hocField := hoField.MemberFields["Configured"]
-	require.NotNil(hocField)
-	require.NotNil(hocField.ShapeRef)
+	prependField := crd.Fields["ResourcesVPCConfig.SecurityGroupIDs"]
+	require.NotNil(prependField)
+	require.NotNil(prependField.ShapeRef)
+
+	require.True(
+		strings.HasPrefix(prependField.GetDocumentation(), "// !!! Let's take it from the top"),
+	)
+
+	appendField := crd.Fields["RoleARN"]
+	require.NotNil(appendField)
+	require.NotNil(appendField.ShapeRef)
+
+	require.True(
+		strings.HasSuffix(appendField.GetDocumentation(), "// !!! Hello earthlings! I have come to assume your permissions."),
+	)
+
+	overrideField := crd.Fields["Identity.OIDC.Issuer"]
+	require.NotNil(overrideField)
+	require.NotNil(overrideField.ShapeRef)
 
 	require.Equal(
-		"// If you set this parameter to true, the instance is enabled for hibernation.\n// \n// Default: false\n//\n// XXX extended docs XXX",
-		hocField.GetDocumentation(),
+		"// !!! All your docs has become mine\n"+
+			"// \n"+
+			"// That whitespace is entirely on purpose",
+		overrideField.GetDocumentation(),
 	)
 
 }