Keyfactor / ejbca-cert-manager-issuer

External issuer for cert-manager for EJBCA
Apache License 2.0
3 stars 3 forks source link

feat(helm): use helm release namespace for all namespaced resources #15

Closed BarthV closed 6 months ago

BarthV commented 9 months ago

Explicit namespaced resources namespace value

Current Behavior

Current behavior is to rely on helm install capabilities to set resources namespace "just in time" during the setup phase.

helm install ejbca-cert-manager-issuer ejbca-issuer/ejbca-cert-manager-issuer --namespace ejbca-issuer-system --create-namespace

This is working pretty well

The problem

The problem is that the namespace is not applied if we're doing the same thing using helm template :

helm template ejbca-cert-manager-issuer deploy/charts/ejbca-cert-manager-issuer --namespace ejbca-issuer-system --create-namespace > no-ns.yaml
---
# Source: ejbca-cert-manager-issuer/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: ejbca-cert-manager-issuer 
< ! MISSING NAMESPACE HERE ! >
  labels:
    helm.sh/chart: ejbca-cert-manager-issuer-0.1.0
    app.kubernetes.io/name: ejbca-cert-manager-issuer
    app.kubernetes.io/instance: ejbca-cert-manager-issuer
    app.kubernetes.io/version: "v1.3.1"
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  ...

This is caused by missing namespace value in helm templates for all namespaced resources (example for Deploy resource : https://github.com/Keyfactor/ejbca-cert-manager-issuer/blob/main/deploy/charts/ejbca-cert-manager-issuer/templates/deployment.yaml#L3-L6 )

This PR makes namespace value explicit in every resources that requires it, enabling a nice compatibility with helm template workflow.

Thanks !

svenska-primekey commented 9 months ago

@m8rmclaren Please review.

BarthV commented 6 months ago

@m8rmclaren gentle bump 👼

m8rmclaren commented 6 months ago

@BarthV Excellent point - thank you for pointing this out. I will get this merged in the next release (hopefully today or early next week)