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's SCC should have volumes in alphabetical order #4208

Open
kbreit-insight opened this issue Jul 24, 2024 · 2 comments
Open

Helm chart's SCC should have volumes in alphabetical order #4208

kbreit-insight opened this issue Jul 24, 2024 · 2 comments
Labels
type/bug Something isn't working

Comments

@kbreit-insight
Copy link

Overview of the Issue

The list of volumes at

should be in alphabetical order. The problem this causes is I am deploying Consul via helm chart in ArgoCD and it won't fully synchronize because it's expecting the hostPath to be at the bottom. However, Kubernetes appears to sort them as here's the current resource section:

volumes:
- configMap
- downwardAPI
- emptyDir
- hostPath
- persistentVolumeClaim
- projected
- secret

Reproduction Steps

Deploy Consul on OpenShift with the CNI enabled. Optionally, deploy via Argo but that should only show it's being applied in a different order.

Expected behavior

I would expect the order to stay the same.

Environment details

OpenShift 4.15 (Kubernetes 1.28.x)
Consul 1.19
Helm chart v1.5.1

@kbreit-insight kbreit-insight added the type/bug Something isn't working label Jul 24, 2024
@zalimeni
Copy link
Member

Thanks for the report, @kbreit-insight !

Based on some initial poking into OpenShift source, I think what you're suggesting makes sense - though the silent forced sorting in OpenShift is (AFAICT) an undocumented behavior, and makes use cases like Argo's diffing unnecessarily brittle. It seems there's an Argo feature request to address cases like this, which could help in the future.

Side note: interestingly, the order consul-k8s is using is almost based on OpenShift behavior, but seems a bit off:

Regardless, I think your suggestion will work given both values are explicitly set in our template together. I'll probably include a comment in case future changes to the conditional might impact the expected order w/ inconsistent post-sort on the OpenShift side. I'll work on reproducing and testing the fix and comment back here when a PR is available.

In the meantime, I believe it's also possible to instruct Argo to ignore differences for specific fields, in case that's helpful as a stopgap.

zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
@zalimeni
Copy link
Member

zalimeni commented Aug 2, 2024

Fix PR: #4227

@kbreit-insight feel free to follow ^ for updates. Assuming it's merged in the next few weeks, it should go out with the next set of consul-k8s patch releases (currently expected ~end of August, though that date isn't yet set in stone). I'll close this issue once the PR is merged, but please comment back w/ any feedback if you run into trouble after testing.

zalimeni added a commit that referenced this issue Aug 2, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 5, 2024
Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
zalimeni added a commit that referenced this issue Aug 5, 2024
openshift: re-order SCC volume list for Argo sync

Due to logic in OpenShift's admissions hook that force-reorders explicit
and implicit entries in this list, our `SecurityContextConstraints`
entries will never successfully sync via tools like ArgoCD, which expect
an exact input and output match when diff'ing.

More details on the problem addressed by this change and potential
future improvements to avoid it in the future can be found in
#4208 (see comments).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants