community icon indicating copy to clipboard operation
community copied to clipboard

Impove resource detection logic in ack-generate

Open jeremyhager opened this issue 6 months ago • 1 comments

Is your feature request related to a problem?

  • The current method of "finding" resources to be managed is by searching for Prefix keywords, which I believe is in this function.

The problem, however, is when there are resources that can be managed but aren't detected. Very specifically the ACK controller for Amazon Cognito Identity Provider cannot detect the Admin* methods, such as AdminCreateUser. This is because the search for managed resources is using strings.HasPrefix, example looking for the prefix Create here, but AdminCreateUser isn't detected since it starts with Admin.

Describe the solution you'd like

  • The ability to detect if a resource API/endpoint has Create within it, rather than start with it.

From my (very limited) testing, this could be achieved by using strings.Contains and strings.Replace.

Here is an example that I have of the changes that may be relevant: https://github.com/jeremyhager/ack-code-generator/blob/model--use-contains-for-op/pkg/model/op.go

For example, here are the generated files for AdminCreateUser: https://github.com/jeremyhager/ack-cognitoidentityprovider-controller/commit/bbf3ccf326f9f2210fd65ca140b53debffce2519

Note no tests, and I haven't tested beyond simple generation, so there are likely more pitfalls to be aware of.

Describe alternatives you've considered

  • Manage AdminCreateUser resources through other means (IaaC, scripts, lambdas, etc.)

jeremyhager avatar Jun 24 '25 19:06 jeremyhager

Hello @jeremyhager 👋 Thank you for opening an issue in ACK! A maintainer will triage this issue soon.

We encourage community contributions, so if you're interested in tackling this yourself or suggesting a solution, please check out our Contribution and Code of Conduct guidelines.

You can find more information about ACK on our website.

github-actions[bot] avatar Jun 24 '25 19:06 github-actions[bot]

Hi @jeremyhager , Thanks for raising this issue!

You're absolutely right that the current prefix-based detection logic in GetOpTypeAndResourceNameFromOpID misses operations like AdminCreateUser because they don't start with "Create".

However, we're aware of this limitation and there's already a supported solution: explicit operation configuration in your generator.yaml file.

Solution: Use Explicit Operation Configuration

Instead of modifying the detection logic, you can explicitly map these operations in your generator.yaml.

You can see a real example of this pattern in the API Gateway.

  PutIntegrationResponse:
    operation_type:
      - Create
    resource_name: ApiIntegrationResponse
  DeleteIntegrationResponse:
    operation_type:
      - Delete
    resource_name: ApiIntegrationResponse
  UpdateIntegrationResponse:
    operation_type:
      - Update
    resource_name: ApiIntegrationResponse
  GetIntegrationResponse:
    operation_type:
      - READ_ONE
    resource_name: ApiIntegrationResponse

The code generator processes explicit operation configurations after the automatic detection logic in pkg/model/sdk_api.go. This means your explicit mappings will override any automatic detection.

rushmash91 avatar Jun 25 '25 01:06 rushmash91

Hi @jeremyhager,

Closing this issue. Feel free to reopen if you run into this again. /close

rushmash91 avatar Jun 30 '25 18:06 rushmash91

@rushmash91: Closing this issue.

In response to this:

Hi @jeremyhager,

Closing this issue. Feel free to reopen if you run into this again. /close

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.

ack-prow[bot] avatar Jun 30 '25 18:06 ack-prow[bot]