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

Postbuild substitution should substitute when kustomization have no substitution defined #1011

Open
NFRBEZ opened this issue Nov 15, 2023 · 5 comments
Labels
wontfix This will not be worked on

Comments

@NFRBEZ
Copy link

NFRBEZ commented Nov 15, 2023

Postbuild substitution is not working when there is no substitution in kustomization :

For example, the following kustomization

---
apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: my-app
  namespace: flux-system
spec:
  path: ./apps/staging/my-app
  targetNamespace: my-app
  interval: 1m
  prune: true
  sourceRef:
    kind: GitRepository
    name: flux-system
  healthChecks:
    - apiVersion: helm.toolkit.fluxcd.io/v2beta1
      kind: HelmRelease
      name: my-app
      namespace: my-app

Generate me the following ressource

---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  labels:
    kustomize.toolkit.fluxcd.io/name: my-app
    kustomize.toolkit.fluxcd.io/namespace: flux-system
  name: my-app
  namespace: my-app
spec:
  chart:
    ...
  values:
    metricsServer:
      dnsPolicy: ${dnsPolicy:=ClusterFirst}
      useHostNetwork: ${hostnetwork:="false"}
...

Which result on problem with my release installation
I need to add at least one postBuild argument to make the substitution occurs, even If it is not referred in my ressources
So doing like this :

---
apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: my-app
  namespace: flux-system
spec:
  path: ./apps/staging/my-app
  targetNamespace: my-app
  interval: 1m
  prune: true
  sourceRef:
    kind: GitRepository
    name: flux-system
  healthChecks:
    - apiVersion: helm.toolkit.fluxcd.io/v2beta1
      kind: HelmRelease
      name: my-app
      namespace: my-app
  postBuild:
    substitute:
      foo: bar

will work well :

---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  labels:
    kustomize.toolkit.fluxcd.io/name: my-app
    kustomize.toolkit.fluxcd.io/namespace: flux-system
  name: my-app
  namespace: my-app
spec:
  chart:
    ...
  values:
    metricsServer:
      dnsPolicy: ClusterFirst
      useHostNetwork: "false"
...
@NFRBEZ NFRBEZ changed the title Postbuild substitution is not substituting when kustomization have no substitution defined Postbuild substitution should substitute when kustomization have no substitution defined Nov 15, 2023
@stefanprodan
Copy link
Member

stefanprodan commented Nov 15, 2023

This is by design, substitutions are opt-in as this operation is expensive for users which have thousands of manifests managed by Flux. You need to specify at least one value, even if you rely on defaults.

@stefanprodan stefanprodan added the wontfix This will not be worked on label Nov 15, 2023
@NFRBEZ
Copy link
Author

NFRBEZ commented Nov 16, 2023

Ok, got it
Wouldn't it be clearer to also add a postbuild.enabled field in the schema with a default to false, so we don't have to add a foo value on substitution array to enable substitution ?

This is a small change for a clearer view of it, as we wouldn't have to check on pointed folder if the variable is set somewhere. It would by default apply all default value

@stefanprodan
Copy link
Member

If we add an enabled filed which defaults to false, then we'll break everyone's clusters, as this will skip the replacements even if there are in place, people will have to edit all their Flux Kustomizations to set enabled true. This type of change can't happen since the Kustomization API is v1 GA.

@NFRBEZ
Copy link
Author

NFRBEZ commented Nov 16, 2023

It depend of the implementation, postbuild would be implicit false without anything, enforced to true if enabled flag is set and defined to true if people defined any postbuild substitution in there kustomization. This way, people would not break theirs clusters.

Something there : https://github.com/fluxcd/kustomize-controller/blob/abdfab3dde0036f1712c3d1b6da90b276bace5a1/internal/controller/kustomization_controller.go#L621C33-L621C33

Which would change to something like the following :

if obj.Spec.PostBuild != nil || obj.Spec.PostBuildEnforce {

@stefanprodan
Copy link
Member

That will be very confusing, why would something still apply if it's set to enabled: false, none of Kubernetes API work like this. Just add a dummy substitution like this:

  postBuild:
    substitute:
      enabled: "yes"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants