Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Beside updating the image Flux Image Automation for HelmReleases remove indentation from sequences #3425

Closed
pommelinho opened this issue Feb 15, 2021 · 9 comments
Assignees
Labels
blocked bug flux2 Resolution suggested - already fixed in Flux v2

Comments

@pommelinho
Copy link

Describe the bug
Beside updating the image Flux Image Automation for HelmReleases remove indentation from sequences.
However our Prettier and Yamllint config is configured that there should be an indentation (as we also find this more readable) and so the CI pipeline will fail.
My question:
Is it somehow possible to configure the indentaion behaviour for flux automation controller?
flu-indentation

Additional context

  • Flux version: 1.21.0
  • Kubernetes version: 1.18
  • Git provider: github
  • Container registry provider: AWS ECR
@pommelinho pommelinho added blocked-needs-validation Issue is waiting to be validated before we can proceed bug labels Feb 15, 2021
@yebyen
Copy link
Contributor

yebyen commented Feb 15, 2021

Both forms are valid according to the yaml spec, but some parsers accept only one or another version.

I am sure this is not presently configurable with the current version of flux but I think it would be a pretty simple patch to add this! It is annoying if it breaks your ci.

@pommelinho
Copy link
Author

Yes it is....
Would be nice when this could be fixed/integrated.
Or does anyone how i can configure this in flux 1.21.0

@yebyen
Copy link
Contributor

yebyen commented Feb 15, 2021

Features are not being added in Flux v1 by the core development team at this point, we are maintaining Flux v1 with bug fixes and non-breaking changes only, with priority for security issues during the maintenance period.

I will check before I investigate too deeply in the Flux v1 sources, whether this issue is still present at all in v2. It is possible this problem is solved in the latest version, where image automation has got an overhaul and kyaml setters are used instead of the old method in Flux v1.

Chances are this code is brittle in Flux v1 and would be hard to change in a non breaking way. We hope you can upgrade with us to the latest version of flux (which is now Flux v2 - 0.8)

I am not sure how long it will take me personally to try adding this feature, but unless you are able to contribute a PR for this, I can say it will not likely be ready for the next release of Flux v1, (1.21.2 - which is due out any day now) but it could be a candidate for including in 1.22.0, if a PR is contributed by outside help.

Fixing issues in Flux v1 that are not present in Flux v2 tends to get lower priority for development cycles. If it is important for your business, and you are not able to upgrade for now, we would love to know more about why. Flux v2 strives to be at or above feature parity in all ways, and if there is a regression or lacking feature still preventing you from upgrade, it will be helpful for the development team to know more about that!

@pommelinho
Copy link
Author

Thanks for the response. Yes upgrading to v2 is indeed an option for us. Do you know if this issue is solved there?

@yebyen
Copy link
Contributor

yebyen commented Feb 16, 2021

I will verify and get back to you today about it 👍

@kingdonb
Copy link
Member

So, I went ahead and followed the flux2 image automation guide with an additional step, before this point I made sure to add an indent to my containers definition, so that it looks like yours, like this below with the kyaml image policy setter added:

    spec:
      containers:
        - name: podinfod
          image: ghcr.io/stefanprodan/podinfo:5.0.0 # {"$imagepolicy": "flux-system:podinfo"}
          imagePullPolicy: IfNotPresent
          ports:
          - name: http
            containerPort: 9898
            protocol: TCP
          - name: http-metrics
            containerPort: 9797
            protocol: TCP

The bad news for you is, the behavior is the same in Flux 2 – the whole section is reindented just the same, the image automation does not just modify the line with the kyaml setter on it, we also remove the extra spaces from the whole containers array section.

I am not sure if flux itself is opinionated about how the yaml should be, or if it is another library that Flux depends on to modify the yaml in place which is making this adjustment. I would bet it is the latter. At any rate I can say now, it will not solve this problem for you to simply upgrade to the latest version.

@kingdonb
Copy link
Member

The good news is (after more investigation)...

In Flux v1, this YAML update is handled by a one-off library written by a Flux team member, kubeyaml, that has not seen a new release for over a year at this point. In Flux v2, it is handled by kustomize/kyaml, a library that is in active development sponsored by a CNCF Kubernetes SIG, and they have an open issue that has just recently been reported:

kubernetes-sigs/kustomize#3559

On the balance this means I think the issue is a lot more likely to get resolved in your favor in Flux v2, though by pure coincidence the same issue is actually present in Flux 2 transitively since it is present in kustomize/kyaml for now at least.

@kingdonb kingdonb added blocked and removed blocked-needs-validation Issue is waiting to be validated before we can proceed labels Feb 17, 2021
@pommelinho
Copy link
Author

ok thank you very much

@kingdonb kingdonb self-assigned this Feb 18, 2021
@kingdonb kingdonb added the flux2 Resolution suggested - already fixed in Flux v2 label Feb 18, 2021
@kingdonb
Copy link
Member

I am going to mark this as closed, with the flux2 label, because I think the resolution will come from Flux 2.

This is probably a minor procedural violation because the issue is not actually resolved, but blocking on some external repo. Unfortunately as it won't be possible for this to be fixed in Flux v1, it also is an issue that does not belong here anymore. You're welcome to reopen against fluxcd/kustomize-controller if it hasn't been fixed by the time you're able to upgrade!

Hope this helped! Thanks for using Flux! 👨‍💻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked bug flux2 Resolution suggested - already fixed in Flux v2
Projects
None yet
Development

No branches or pull requests

3 participants