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

Helm Chart: certmanager requires user specify .Values.clustermesh.apiserver.tls.auto.certManagerIssuerRef #22784

Closed
2 tasks done
wigust opened this issue Dec 18, 2022 · 1 comment · Fixed by #22921
Closed
2 tasks done
Assignees
Labels
kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps.

Comments

@wigust
Copy link

wigust commented Dec 18, 2022

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

The Helm Chart validation for clustermesh.apiserver.tls.auto.certManagerIssuerRef is broken.

Running helm template --values val.yaml ./. fails on 1.12.4 and origin/master (d8c7d93) because of validation in install/kubernetes/cilium/templates/validate.yaml:

{{- if and (or .Values.externalWorkloads.enabled .Values.clustermesh.useAPIServer) .Values.clustermesh.apiserver.tls.auto.enabled (eq .Values.clustermesh.apiserver.tls.auto.method "certmanager") }}
  {{- if not .Values.clustermesh.apiserver.tls.auto.certManagerIssuerRef }}
    {{ fail "ClusterMesh TLS certgen method=certmanager requires user specify .Values.clustermesh.apiserver.tls.auto.certManagerIssuerRef" }}
  {{- end }}  
{{- end }}

val.yaml:

rollOutCiliumPods: true

cni:
  exclusive: false
  debug:
    enabled: true

prometheus:
  enabled: true

operator:
  rollOutPods: true
  prometheus:
    enabled: true

kubeProxyReplacement: "strict"

k8sServiceHost: "172.16.102.137"
k8sServicePort: "6443"

bpf:
  masquerade: true

bgp:
  enabled: false
  announce:
    loadbalancerIP: true

ipam:
  mode: "kubernetes"

bgpControlPlane:
  enabled: true

ingressController:
  enabled: true

hubble:
  relay:
    rollOutPods: true
    enabled: true
  ui:
    rollOutPods: true
    enabled: true

bandwidthManager:
  enabled: true

cluster:
  name: "cluster2"
  id: 2

clustermesh:
  name: "cluster2"
  config:
    enable: true
  clusters: ["cluster1", "cluster2"]
  useAPIServer: true
  apiserver:
    tls:
      auto:
        method: "certmanager"

Cilium Version

1.12.4

Kernel Version

5.15.78

Kubernetes Version

1.25.4

Sysdump

No response

Relevant log output

helm template --values val.yaml ./.

ClusterMesh TLS certgen method=certmanager requires user specify .Values.clustermesh.apiserver.tls.auto.certManagerIssuerRef

Anything else?

According to the default values.yaml documentation string the user does not need to provide clustermesh.apiserver.tls.auto.certManagerIssuerRef in which case it will be generated.

# [Example]
# certManagerIssuerRef:
# group: cert-manager.io
# kind: ClusterIssuer
# name: ca-issuer
# -- certmanager issuer used when hubble.tls.auto.method=certmanager.
# If not specified, a CA issuer will be created.
certManagerIssuerRef: {}

Code of Conduct

  • I agree to follow this project's Code of Conduct
@wigust wigust added kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps. labels Dec 18, 2022
Shunpoco added a commit to Shunpoco/cilium that referenced this issue Jan 4, 2023
Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: cilium#22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
@Shunpoco
Copy link
Contributor

Shunpoco commented Jan 4, 2023

I caught the same issue, so I made a PR to fix this problem.
@wigust thank you for your issuing!

ldelossa pushed a commit that referenced this issue Jan 23, 2023
Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: #22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
sayboras pushed a commit to sayboras/cilium that referenced this issue Jan 24, 2023
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: cilium#22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
aanm pushed a commit that referenced this issue Jan 24, 2023
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: #22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
michi-covalent pushed a commit to ldelossa/cilium that referenced this issue Jan 25, 2023
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: cilium#22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
michi-covalent pushed a commit that referenced this issue Jan 26, 2023
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: #22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
YutaroHayakawa pushed a commit to YutaroHayakawa/cilium that referenced this issue Mar 17, 2023
[ upstream commit bc2ed14 ]

Because the helm chart generates cert manager issuers and attaches them
to certificates, we have to remove validations which fail if we don't
specify certManagerIssuerRef.

Fixes: cilium#22784

Signed-off-by: Shunsuke Tokunaga <tkngsnsk313320@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants