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

feat: default ControllerConfiguration apiVersion and kind in helm #7126

Conversation

ThatsMrTalbot
Copy link
Contributor

@ThatsMrTalbot ThatsMrTalbot commented Jun 25, 2024

Pull Request Motivation

This makes it easier to configure cert-manager using Helm by defaulting config.apiVersion and config.kind.

For example, enabling gateway api goes from:

--set config.apiVersion="controller.config.cert-manager.io/v1alpha1" \
--set config.kind="ControllerConfiguration" \
--set config.enableGatewayAPI=true

to:

--set config.enableGatewayAPI=true

Kind

feature

Release Note

Make it easier to configure cert-manager using Helm by defaulting `config.apiVersion` and `config.kind` within the Helm chart.

@cert-manager-prow cert-manager-prow bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. area/deploy Indicates a PR modifies deployment configuration needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 25, 2024
@ThatsMrTalbot ThatsMrTalbot force-pushed the feat/helm-default-config-apiversion-and-kind branch from 5725f07 to 2d0d4bf Compare June 25, 2024 10:33
@cert-manager-prow cert-manager-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 25, 2024
@ThatsMrTalbot ThatsMrTalbot force-pushed the feat/helm-default-config-apiversion-and-kind branch from 2d0d4bf to 1393dbb Compare June 25, 2024 10:35
@@ -1,6 +1,7 @@
{{- if .Values.cainjector.config -}}
{{- $_ := .Values.cainjector.config.apiVersion | required ".Values.cainjector.config.apiVersion must be set !" -}}
{{- $_ := .Values.cainjector.config.kind | required ".Values.cainjector.config.kind must be set !" -}}
{{- $config := .Values.cainjector.config -}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some comments in the values.yaml file about backwards compatibility etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment in the values.yaml for each config block describing the behaviour and how to pin to a version

@ThatsMrTalbot ThatsMrTalbot force-pushed the feat/helm-default-config-apiversion-and-kind branch from 1393dbb to dcc44ef Compare June 25, 2024 10:55
@cert-manager-prow cert-manager-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 25, 2024
Signed-off-by: Adam Talbot <adam.talbot@venafi.com>
@ThatsMrTalbot ThatsMrTalbot force-pushed the feat/helm-default-config-apiversion-and-kind branch from dcc44ef to e30ad68 Compare June 25, 2024 10:58
@maelvls
Copy link
Member

maelvls commented Jun 25, 2024

Hey, thanks for writing this feature so quickly! For context, this is useful for #7121.

I tested this with

$ gh pr checkout 7126
$ make helm-chart
$ helm upgrade --install cert-manager _bin/helm/cert-manager --create-namespace --namespace cert-manager \ 
  --set installCRDs=true \
  --set config.enableGatewayAPI=true

$ kubectl get cm -n cert-manager cert-manager -oyaml
apiVersion: v1
data:
  config.yaml: |
    apiVersion: controller.config.cert-manager.io/v1alpha1
    enableGatewayAPI: true
    kind: ControllerConfiguration

All good!

/lgtm
/approve

@cert-manager-prow cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2024
@cert-manager-prow
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls

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:

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

@cert-manager-prow cert-manager-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2024
@cert-manager-prow cert-manager-prow bot merged commit 1b9c02e into cert-manager:master Jun 25, 2024
5 of 6 checks passed
@wallrj
Copy link
Member

wallrj commented Jul 24, 2024

/kind feature

@cert-manager-prow cert-manager-prow bot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/deploy Indicates a PR modifies deployment configuration dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants