support field custom documentation strings

[jaypipes]

Adds support for extending the documentation strings for fields and setting docstrings for custom fields.

A new FieldConfig.Documentation configuration option is now supported. When present, this string extends any documentation provided for the existing field by the doc-2.json files. If no such documentation is provided by the existing field OR if the field is a custom field, the FieldConfig.Documentation configuration option is used for the field's docstring in the Go type files.

Example usage for an existing nested field (from a hypothetical ec2 generator.yaml file). The
LaunchTemplateData.HibernationOptions.Configured field has a docstring already that contains the following:

// If you set this parameter to true, the instance is enabled for hibernation.
//
// Default: false

Setting the generator.yaml file to the following:

resources:
  LaunchTemplate:
    fields:
      LaunchTemplateData.HibernationOptions.Configured:
        documentation: XXX extended docs XXX

would cause the field's Documentation string to look like this:

// If you set this parameter to true, the instance is enabled for hibernation.
//
// Default: false
//
// XXX extended docs XXX",

Issue aws-controllers-k8s/community#1223

Signed-off-by: Jay Pipes jaypipes@gmail.com

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

[ack-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jaypipes
To complete the pull request process, please assign jljaco after the PR has been reviewed.
You can assign the PR to them by writing /assign @jljaco in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ack-bot]

Hi @jaypipes. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jaypipes]

@RedbackThomson @jljaco @a-hilaly please note the changes to comment formatting is from using go1.19 gofmt changes...

[RedbackThomson]

I have some concerns about how unruly the generator.yaml file will become if there are going to be lots of custom fields. (cough S3). Rather, I'd prefer if we could track the documentation in a separate file and instead add pointers to that file on a per-resource level. That way documentation could stay within the apis/v1alpha1 directory? So either we could have:

resource:
  Cluster:
    documentation_file: docs/cluster.yaml

and then a apis/v1alpha1/docs/cluster.yaml file containing:

field1: >
  Multi-line
  Documentation
  Here
field2: ...

Or we could have a per-controller version of that where we define a top-level config in the generator.yaml like so:

documentation_file: docs.yaml

Then at apis/v1alpha1/docs.yaml

Cluster:
  field1: ...
Addons:
  field1: ...

[jaypipes]

I have some concerns about how unruly the generator.yaml file will become if there are going to be lots of custom fields. (cough S3). Rather, I'd prefer if we could track the documentation in a separate file and instead add pointers to that file on a per-resource level. That way documentation could stay within the apis/v1alpha1 directory? So either we could have:

resource:
  Cluster:
    documentation_file: docs/cluster.yaml

and then a apis/v1alpha1/docs/cluster.yaml file containing:

field1: >
  Multi-line
  Documentation
  Here
field2: ...

Or we could have a per-controller version of that where we define a top-level config in the generator.yaml like so:

documentation_file: docs.yaml

Then at apis/v1alpha1/docs.yaml

Cluster:
  field1: ...
Addons:
  field1: ...

@RedbackThomson I guess I see where you're coming from if the controller has a LOT of custom fields. s3-controller only actually has 4 fields though :) So not too onerous to have the docs in a single generator.yaml file IMHO.

Then again, I do like your suggestion about having a documentation_file top-level configuration option. I could also see a possible doc_from configuration option where, say for custom fields, you tell ack-generate where to find the documentation for a particular field (basically give it the Shape and Member path in the doc-2.json file to use)

[RedbackThomson]

@RedbackThomson I guess I see where you're coming from if the controller has a LOT of custom fields. s3-controller only actually has 4 fields though :) So not too onerous to have the docs in a single generator.yaml file IMHO.

Then again, I do like your suggestion about having a documentation_file top-level configuration option. I could also see a possible doc_from configuration option where, say for custom fields, you tell ack-generate where to find the documentation for a particular field (basically give it the Shape and Member path in the doc-2.json file to use)

Both sound great! How about:

resources:
  Cluster:
    fields:
      MyCustomField:
        documentation: # Supports XOR `from_sdk` and `from_custom`
          from_sdk:
            operation: ...
            path: ...
          from_custom: ... 

[a-hilaly]

I'm a bit mixed regarding using new files different than generator.yaml, but i also agree that adding documentation in the "fields" config can be super verbose, how about doing something similar to what's below?

resources:
  User:
    fields:
       newField:
          documentation:
             from: X
  Group:
     fields:
        newField:
           documentation:
              from: X

documentation:
  X: is an option that does ... # This can be reused many times (thinking resources that have similar fields like ARN etc...)
  Y: is an option that does ...

[a-hilaly]

/ok-to-test

[a-hilaly]

/test all

[a-hilaly]

/retest

[a-hilaly]

👍

[RedbackThomson]

/retest

[RedbackThomson]

Even though I still think there should be some UX changes to this feature, I am going to merge this because I want to use it to inject deprecation notices into fields.
/lgtm

[ack-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, jaypipes, RedbackThomson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [A-Hilaly,RedbackThomson,jaypipes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ack-prow]

@jaypipes: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ec2-controller-test	5e1b51a	link	unknown	/test ec2-controller-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.