Allow member fields from other operations in spec

[RedbackThomson]

Description of changes:
Allow the use of member fields contained within other API operations within a resource's spec. For example, LoggingStatus for S3 is only in the PutBucketLogging operation, but should be settable from spec.

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

[vijtrip2]

Is this a PR in progress?
From the code changes, I could not relate how these change will allow mentioning spec fields from another operation. Can you please elaborate for me?
Also No unit tests added?

[RedbackThomson]

Is this a PR in progress?

Nope. This is just a small PR.

From the code changes, I could not relate how these change will allow mentioning spec fields from another operation. Can you please elaborate for me?

I want to add LoggingStatus as a spec field for S3. To do this, I would add a new field in the configuration and set the operation to PutBucketLogging with path LoggingStatus. When I go to generate this code, the generator will add the LoggingStatus field to the CRD, but the underlying go type for that field (and any of its members) are not added to the type defs. This is because this part of the generator ensures that all type defs are in the input or output of some API - but in this case PutBucketLogging is neither.

Also No unit tests added?

I don't see any unit tests for the type defs methods. Any pointers?

[vijtrip2]

Is this a PR in progress?

Nope. This is just a small PR.

From the code changes, I could not relate how these change will allow mentioning spec fields from another operation. Can you please elaborate for me?

I want to add LoggingStatus as a spec field for S3. To do this, I would add a new field in the configuration and set the operation to PutBucketLogging with path LoggingStatus. When I go to generate this code, the generator will add the LoggingStatus field to the CRD, but the underlying go type for that field (and any of its members) are not added to the type defs. This is because this part of the generator ensures that all type defs are in the input or output of some API - but in this case PutBucketLogging is neither.

Also No unit tests added?

I don't see any unit tests for the type defs methods. Any pointers?

Thanks for explaining. We should ideally make an effort to increase test coverage of our codebase (maybe a collective effort as a team in coming sprints) but it's definitely out of scope for this PR.

LGTM. I'll let one more team member review this before merging.

[jaypipes]

@RedbackThomson please see my inline comment.

Also, to unit test this functionality, simply add the generator config changes you are testing out for the S3 controller's update code paths to the testdata/ generator yaml for S3:

https://github.com/aws-controllers-k8s/code-generator/blob/5a40e37032506f12a07b7de12a6005b525d13ad9/pkg/testdata/models/apis/s3/0000-00-00/generator.yaml

and add a test for the now-expected new Spec field for Logging in this unit test file:

code-generator/pkg/model/model_s3_test.go

Line 70 in 5a40e37

expSpecFieldCamel := []string{

and a test for the now-expected LoggingStatus type defs in the same model_s3_test.go file. You can use the testutil.GetTypeDefByName utility function from here:

code-generator/pkg/testutil/get.go

Line 45 in 5a40e37

func GetTypeDefByName(

[RedbackThomson]

Cleaned up unnecessary methods and added unit tests for the S3 Logging type

[a-hilaly]

👍

[jaypipes]

Bingo. ++

[jaypipes]

👍

[jaypipes]

/lgtm

[ack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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,vijtrip2]

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