cert-manager / trust-manager

trust-manager is an operator for distributing trust bundles across a Kubernetes cluster.
https://cert-manager.io/docs/projects/trust-manager/
Apache License 2.0
243 stars 65 forks source link

refactor: use generated Bundle API apply configurations #239

Closed erikgb closed 5 months ago

erikgb commented 9 months ago

This PR is an alternative to https://github.com/cert-manager/trust-manager/pull/237 that also uses the generated Bundle API apply configurations introduced in https://github.com/cert-manager/trust-manager/pull/217, but uses the established pattern for SSA-patching resources from apply configurations.

When this PR is hopefully merged, I suggest moving the generated apply configurations to an exported package - which will make them available to any user pulling in this project as a dependency.

jetstack-bot commented 9 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from erikgb. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/cert-manager/trust-manager/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
erikgb commented 9 months ago

@inteon The failing test now seems to be a real flake. I got it a couple of times yesterday as well. Any idea what might be the root cause?

SgtCoDFish commented 9 months ago

Capturing an important point of discussion from the standup on 2023-11-21: This PR would be a no-op in practice (i.e., this shouldn't change any functionality for anyone running trust-manager).

The motivation here is dogfooding - we think clients should use the apply configurations when interacting with trust-manager resources, and so there's value in us doing that (so everyone's working with the same API)

erikgb commented 6 months ago

/cc @SgtCoDFish

erikgb commented 5 months ago

It seems like there is no desire to use applyconfigurations in the maintainer team, closing. I'll create a PR to remove the generation of the ACs, as there is no point in having them if we're not going to use them. :-)