Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it possible to inject new fields during code generation? #881

Open
ulucinar opened this issue Jul 29, 2021 · 5 comments
Open

Is it possible to inject new fields during code generation? #881

ulucinar opened this issue Jul 29, 2021 · 5 comments
Labels
area/code-generation Issues or PRs as related to controllers or docs code generation area/crossplane Issues or PRs related to crossplane kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@ulucinar
Copy link

Is your feature request related to a problem?
Similar to being able to ignore certain resources or field paths during code generation by specifying them under ignore.resource_names or ignore.field_paths in a generator_config.yaml file, is it/would it be possible to introduce new fields at specified paths with specified types & tags?

For example, while using the crossplane pipeline, we can have referencer fields that enable us to refer to another custom resource via a label selector or its name. The use case would be to introduce these extra "referencer" fields during code generation.

Describe the solution you'd like
To be able to define extra fields at specified paths with specified types (possibly a type unknown to the code-generator pipeline) and with specified tags.

Describe alternatives you've considered
One alternative might be to have the code-generator ignore the type/field which we need to expand and manually define those types/fields. But we would then not be utilizing the code generator. There are also other alternatives for the problem at hand in the crossplane context but it seems to be extending the code-generator with such functionality (if not already provided) will open up other possibilities.

For more context, the original context of the problem is a CloudFront Distribution can have multiple CacheBehaviors, each of which can refer to a (possibly distinct) CachePolicy. Now if we have a Distribution CR and multiple CachePolicy CRs to which we would like to refer to from the Distribution, I think it makes most sense to introduce the references fields (e.g., CachePolicy CR name and/or label selector) in the generated CacheBehavior struct. What I'm planning to implement for this PR at the moment is to introduce an index field that will point to the CacheBehavior object in the cacheBehaviors array for which a reference is being defined (assuming cachePolicyID is optional for a CacheBehavior).

Thank you very much.

@ulucinar ulucinar added the kind/enhancement Categorizes issue or PR as related to existing feature enhancements. label Jul 29, 2021
@jaypipes
Copy link
Collaborator

Hi @ulucinar! This is a great feature suggestion. Can you make it to the next ACK community meeting on Zoom on Monday @12pm EDT? Details are here:

https://github.com/aws-controllers-k8s/community/#community-meeting

Would be great to discuss this feature request with you. We can currently inject fields into a CR's Spec or Status that are in other AWS SDK shapes but not yet have the ability to override the Go type that is inferred by looking at that Shape's member fields. That said, I'd be open to adding this type of configuration to the generator conf.

@jaypipes jaypipes added area/code-generation Issues or PRs as related to controllers or docs code generation area/crossplane Issues or PRs related to crossplane labels Jul 29, 2021
@ulucinar
Copy link
Author

ulucinar commented Aug 9, 2021

Hi @jaypipes,
I was not available last week but I will try to do it today. Thank you very much.

@jaypipes
Copy link
Collaborator

jaypipes commented Aug 9, 2021

As mentioned in the community meeting this morning, we should refine this GH issue to talk specifically about new fields for a CR that do not appear in any shape from the aws-sdk-go API model files. I will work on that refinement later today or tomorrow.

@RedbackThomson
Copy link
Contributor

I'm interested in taking this on, as an effort to complete the S3 bucket specification. My early implementation would probably accept a string, as part of the generator config, that specifies the Go type. I'm also going to assume this type is in the resource's package, which is where we typically place the hooks. This removes the need to handle custom importing, which complicates things quite a lot.

@RedbackThomson RedbackThomson self-assigned this Aug 26, 2021
@RedbackThomson RedbackThomson added this to In progress in ACK Core Team Sep 8, 2021
ack-bot pushed a commit to aws-controllers-k8s/code-generator that referenced this issue Oct 21, 2021
Issue #, if available: aws-controllers-k8s/community#881

Description of changes:
Supports creating custom list and map fields where the member shapes of those fields already exist in the SDK.
Rather than supplying a `from` in the field config, you can now supply a `custom_field` property which requires
either `list_of` or `map_of` with the name of an API shape as the value.

An example of generating a list (for an S3 Bucket):
```yaml
resources:
  Bucket:
    fields:
      AnalyticsConfigurations:
        custom_field:
          list_of: AnalyticsConfiguration
```

Produces the following:
```golang
AnalyticsConfigurations []*AnalyticsConfiguration `json:"analyticsConfigurations,omitempty"`
```

An example of generating a map (for an S3 Bucket):
```yaml
resources:
  Bucket:
    fields:
      AnalyticsConfigurations:
        custom_field:
          map_of: AnalyticsConfiguration
```

Produces the following:
```golang
AnalyticsConfigurations map[string]*AnalyticsConfiguration `json:"analyticsConfigurations,omitempty"` 
```

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

I have merged aws-controllers-k8s/code-generator#222 , which provides an initial implementation of new fields. It only supports new list or map fields of existing AWS SDK shapes, though.

Once we have #545 designed and implemented, we can look into also supporting your use case:

The use case would be to introduce these extra "referencer" fields during code generation.

@RedbackThomson RedbackThomson moved this from In progress to Done in ACK Core Team Oct 27, 2021
@ack-bot ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 26, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jan 27, 2022
@jaypipes jaypipes added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-generation Issues or PRs as related to controllers or docs code generation area/crossplane Issues or PRs related to crossplane kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
No open projects
Status: Todo
Development

No branches or pull requests

4 participants