Make all Deployment related Helm values global

[wallrj]

Some Deployment related Helm chart values are currently nested under app.webhook, which is confusing because the webhook and the controller are both part of a single Deployment.

It's inconsistent with other Deployment related global Helm values in this chart such as resources and volumes and volumeMounts.

It is also inconsistent with the same Helm values in trust-manager, which are global.

• fixes #13 - Allow node selection based on nodeSelector, tolerations, affinities and topologySpreadConstraints trust-manager#117

https://github.com/cert-manager/trust-manager/blob/34bb21e768695b4503e8e831e99f5d5ffb53f441/deploy/charts/trust-manager/values.yaml#L110-L135

Testing

Deprecation warnings

# values.yaml
image:
  tag: v0.12.1

replicaCount: 2

podDisruptionBudget:
  enabled: true

topologySpreadConstraints:
- maxSkew: 2
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
  labelSelector:
    matchLabels:
      app.kubernetes.io/name: cert-manager-approver-policy
      app.kubernetes.io/instance: cert-manager-approver-policy

app:
  webhook:

    hostNetwork: true
    dnsPolicy: ClusterFirstWithHostNet

    nodeSelector:
      kubernetes.io/os: linux


    affinity:
      nodeAffinity:
       requiredDuringSchedulingIgnoredDuringExecution:
         nodeSelectorTerms:
         - matchExpressions:
           - key: node-restriction.kubernetes.io/reserved-for
             operator: In
             values:
             - platform

    tolerations:
    - key: node-restriction.kubernetes.io/reserved-for
      operator: Equal
      value: platform

$ helm upgrade cert-manager-approver-policy deploy/charts/approver-policy  --values values.yaml  --namespace venafi --create-namespace --install
Release "cert-manager-approver-policy" has been upgraded. Happy Helming!
NAME: cert-manager-approver-policy
LAST DEPLOYED: Thu Feb 29 17:02:22 2024
NAMESPACE: venafi
STATUS: deployed
REVISION: 5
TEST SUITE: None
NOTES:
⚠️  WARNING: The Helm value `.app.webhook.affinity` is deprecated. Use `.affinity` instead.
⚠️  WARNING: The Helm value `.app.webhook.nodeSelector` is deprecated. Use `.nodeSelector` instead.
⚠️  WARNING: The Helm value `.app.webhook.tolerations` is deprecated. Use `.tolerations` instead.
⚠️  WARNING: The Helm value `.app.webhook.hostNetwork` is deprecated. Use `.hostNetwork` instead.
⚠️  WARNING: The Helm value `.app.webhook.dnsPolicy` is deprecated. Use `.dnsPolicy` instead.

CHART NAME: cert-manager-approver-policy
CHART VERSION: v0.0.0
APP VERSION: v0.0.0

cert-manager-approver-policy is a cert-manager project.

If you're a new user, we recommend that you read the [cert-manager Approval Policy documentation] to learn more.

[cert-manager Approval Policy documentation]: https://cert-manager.io/docs/policy/approval/

Without values

$ helm template deploy/charts/approver-policy/ | grep -C 5 -e affinity -e tolerations -e nodeSelector -e hostNetwork -e dnsPolicy
        securityContext:
          allowPrivilegeEscalation: false
          capabilities: { drop: ["ALL"] }
          readOnlyRootFilesystem: true

      hostNetwork: false
      dnsPolicy: ClusterFirst
---
# Source: cert-manager-approver-policy/templates/webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:

Using global values

$ helm template deploy/charts/approver-policy/ --set-json 'affinity={"foo": "bar"}' --set-json='tolerations=[{}]' --set-json='nodeSelector={"foo": "bar"}' --set hostNetwork=true --set dnsPolicy=foo | grep -C 5 -e affinity -e
tolerations -e nodeSelector -e hostNetwork -e dnsPolicy
        securityContext:
          allowPrivilegeEscalation: false
          capabilities: { drop: ["ALL"] }
          readOnlyRootFilesystem: true

      hostNetwork: true
      dnsPolicy: foo
      nodeSelector:
        foo: bar
      tolerations:
        - {}
      affinity:
        foo: bar
---
# Source: cert-manager-approver-policy/templates/webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration

Using deprecated values

$ helm template deploy/charts/approver-policy/ --set-json 'app.webhook.affinity={"foo": "bar"}' --set-json='app.webhook.tolerations=[{}]' --set-json='app.webhook.nodeSelector={"foo": "bar"}' --set app.webhook.hostNetwork=true --set app.webhook.dnsPolicy=foo | grep -C 5 -e affinity -e tolerations -e nodeSelector -e hostNetwork -e dnsPolicy
        securityContext:
          allowPrivilegeEscalation: false
          capabilities: { drop: ["ALL"] }
          readOnlyRootFilesystem: true

      hostNetwork: true
      dnsPolicy: foo
      nodeSelector:
        foo: bar
      tolerations:
        - {}
      affinity:
        foo: bar
---
# Source: cert-manager-approver-policy/templates/webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration

[wallrj]

@inteon How can I fix this error?

• https://prow.build-infra.jetstack.net/view/gs/jetstack-logs/pr-logs/pull/cert-manager_approver-policy/387/pull-cert-manager-approver-policy-verify/1763226604642766848

/home/prow/go/src/github.com/cert-manager/approver-policy/_bin/tools/helm-tool lint -i deploy/charts/approver-policy/values.yaml -d deploy/charts/approver-policy/templates -e deploy/charts/approver-policy/values.linter.exceptions
value missing from templates: nodeSelector
value missing from templates: tolerations
value missing from templates: affinity
Could not lint: values.yaml and templates are not in sync
make: *** [make/_shared/helm///helm.mk:106: verify-helm-values] Error 1

Discussed with @inteon on slack and we think it's a problem with helm-tool lint. I've added some exceptions for the timebeing.

[inteon]

@wallrj does it make sense that hostNetwork and dnsPolicy remain as options under the webhook value?

[wallrj]

Yeah, I noticed those. Probably not. I'll move those too.

[wallrj]

Done.

[wallrj]

@inteon PTAL

• I've also made hostNetwork and dnsPolicy global as you suggested and retested. See PR description for new output.
• The (or ...) now checks the app.webhook scoped values first, because dnsPolicy has a string default in the default values file, so we need to check the deprecated path to the value because users who have overridden the value will be using that. They will hopefully see the warning notice in the NOTES and change their values files....and then sometime in the future we can remove the old .app.webhook values.

[wallrj]

The helm-tool lint command did not have problems with hostNetwork or dnsPolicy for some reason. Perhaps because those variables are not used in with ... statements in the templates.

[wallrj]

/retest

[wallrj]

https://github.com/cert-manager/approver-policy/blob/f2034b48c02af8517ea92f7e947f30b804197706/deploy/charts/approver-policy/README.md#appwebhookhostnetwork--bool

[inteon]

Thanks @wallrj. I verified a few of the values, and it seems to work as expected.
/approve
/lgtm

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon

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 [inteon]

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