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

fix: Add update logic to handle addition of PSS to deployments #1533

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

svghadi
Copy link
Collaborator

@svghadi svghadi commented Sep 2, 2024

What type of PR is this?

/kind bug

What does this PR do / why we need it:

#1288 and #1493 introduced Pod Security Standards (PSS) to ensure all Argo CD component pods comply with the restricted security policy. However, these changes only apply to new installations.

This PR addresses the gap by adding logic to update existing installations. It ensures that the necessary securityContext is applied to existing pods and automatically reconciles any manual changes back to the intended configuration.

Which issue(s) this PR fixes:

Closes #1492

How to test changes / Special notes to the reviewer:

I performed an upgrade test as follows:

  1. Ran make run with the v0.11.0 tag.
  2. Created an ArgoCD CR.
  3. Stopped make run and switched to this PR branch.
  4. Ran make run again.

The following command should return no warnings:

# Example of a non-compliant namespace
$ kubectl label --overwrite --dry-run=server ns test pod-security.kubernetes.io/enforce=restricted
Warning: existing pods in namespace "test" violate the new PodSecurity enforce level "restricted:latest"
namespace/test labeled (server dry run)

# Example of a compliant namespace
$ kubectl label --overwrite --dry-run=server ns test pod-security.kubernetes.io/enforce=restricted
namespace/test labeled (server dry run)

@saumeya
Copy link
Collaborator

saumeya commented Sep 3, 2024

Good solution for upgrade testing @svghadi

@@ -1515,3 +1514,18 @@ func (r *ReconcileArgoCD) reconcileKeycloak(cr *argoproj.ArgoCD) error {

return nil
}

func restrictedContainerSecurityContext() *corev1.SecurityContext {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we creating a function for security context only for keycloak resource?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I wanted to create a common function for all deployments but realized that some have slightly different securityContext configurations. So to keep things simpler, didn't proceed with it. We can revisit this when we work on refactoring again.

Copy link
Collaborator

@saumeya saumeya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the upgrade scenario from 0.11 to this branch and works as expected.
LGTM
Thanks @svghadi

@iam-veeramalla
Copy link
Collaborator

LGTM @svghadi, Thanks for the PR.

@iam-veeramalla iam-veeramalla merged commit db22b50 into argoproj-labs:master Sep 4, 2024
7 checks passed
@svghadi svghadi deleted the pss-update branch September 10, 2024 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v0.11.0] argocd-operator deployment fails with PodSecurity "restricted:latest", seccompProfile missing
3 participants