Skip to content

Conversation

@Mangaal
Copy link
Contributor

@Mangaal Mangaal commented Feb 19, 2025

What type of PR is this?
In OpenShift clusters using the GitOps Operator, the argocd-redis ServiceAccount currently inherits the nonroot-v2 SecurityContextConstraints (SCC). This grants more privileges than the standard restricted-v2 SCC, allowing other pods in the namespace to use this ServiceAccount with elevated permissions.

This PR updates both cluster-scoped and namespace-scoped ArgoCD instances to use the restricted-v2 SCC for the Redis ServiceAccount

/kind bug

Verification: No Issues Running with restricted-v2 SCC

To confirm that using restricted-v2 SCC does not cause any issues, we checked an ArgoCD instance in another namespace (test-argocd), where all components are running as expected under restricted-v2:

$ oc get po -n test-argocd
NAME                                       READY   STATUS    RESTARTS   AGE
argocd-application-controller-0            1/1     Running   0          99m
argocd-redis-ha-haproxy-7c8599bb57-kdw4q   1/1     Running   0          99m
argocd-redis-ha-server-0                   2/2     Running   0          99m
argocd-redis-ha-server-1                   2/2     Running   0          98m
argocd-redis-ha-server-2                   2/2     Running   0          97m
argocd-repo-server-8684889b98-z88ts        1/1     Running   0          99m
argocd-server-59c5859dfd-spcj2             1/1     Running   0          99m

Checking the SCC assigned to the Redis components:

$ oc get pod argocd-redis-ha-haproxy-7c8599bb57-kdw4q -n test-argocd -o jsonpath="{.metadata.annotations.openshift\.io/scc}"
restricted-v2

$ oc get pod argocd-redis-ha-server-0 -n test-argocd -o jsonpath="{.metadata.annotations.openshift\.io/scc}"
restricted-v2

Fixes #?
#1662

Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
@Mangaal Mangaal changed the title Restrict ArgoCD Redis ServiceAccount to restricted-v2 SCC [wip]Restrict ArgoCD Redis ServiceAccount to restricted-v2 SCC Feb 19, 2025
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
@Mangaal Mangaal changed the title [wip]Restrict ArgoCD Redis ServiceAccount to restricted-v2 SCC Restrict ArgoCD Redis ServiceAccount to restricted-v2 SCC Feb 19, 2025
},
}

// Need additional policy rules if we are running on openshift, else the stateful set won't have the right

Choose a reason for hiding this comment

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

Most probably this comment needs to be rewritten (mentioning that on OpenShift we make sure the Pods are running under the least allowed privilege) or dropped altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @EmmanuelKasper , I have added the suggested comment

Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

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

A nit, rest all looks good. Thanks @Mangaal

Comment on lines 470 to 535
if IsOpenShiftCluster() {
deploy.Spec.Template.Spec.Containers = []corev1.Container{{
Args: getArgoRedisArgs(useTLS),
Image: getRedisContainerImage(cr),
ImagePullPolicy: corev1.PullAlways,
Name: "redis",
Ports: []corev1.ContainerPort{
{
ContainerPort: common.ArgoCDDefaultRedisPort,
},
},
},
Resources: getRedisResources(cr),
Env: env,
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: boolPtr(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Resources: getRedisResources(cr),
Env: env,
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: boolPtr(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
RunAsNonRoot: boolPtr(true),
SeccompProfile: &corev1.SeccompProfile{
Type: "RuntimeDefault",
},
},
RunAsNonRoot: boolPtr(true),
RunAsUser: int64Ptr(999),
SeccompProfile: &corev1.SeccompProfile{
Type: "RuntimeDefault",
VolumeMounts: []corev1.VolumeMount{
{
Name: common.ArgoCDRedisServerTLSSecretName,
MountPath: "/app/config/redis/tls",
},
},
},
VolumeMounts: []corev1.VolumeMount{
{
Name: common.ArgoCDRedisServerTLSSecretName,
MountPath: "/app/config/redis/tls",
}}
} else {
deploy.Spec.Template.Spec.Containers = []corev1.Container{{
Args: getArgoRedisArgs(useTLS),
Image: getRedisContainerImage(cr),
ImagePullPolicy: corev1.PullAlways,
Name: "redis",
Ports: []corev1.ContainerPort{
{
ContainerPort: common.ArgoCDDefaultRedisPort,
},
},
},
}}
Resources: getRedisResources(cr),
Env: env,
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: boolPtr(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
},
},
RunAsNonRoot: boolPtr(true),
RunAsUser: int64Ptr(999),
SeccompProfile: &corev1.SeccompProfile{
Type: "RuntimeDefault",
},
},
VolumeMounts: []corev1.VolumeMount{
{
Name: common.ArgoCDRedisServerTLSSecretName,
MountPath: "/app/config/redis/tls",
},
},
}}
}
Copy link
Collaborator

@svghadi svghadi Feb 19, 2025

Choose a reason for hiding this comment

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

The only thing that changes between k8s and openshift is

RunAsUser:    int64Ptr(999),

Can you simplify the if? Maybe try using deploy.Spec.Template.Spec.SecurityContext like redis ha statefulset or use a var for securitycontext value and set that in deploy.Spec.Template.Spec.Containers[*]

Also can we try changing the user to 1000 or 1001 if the redis image supports it on k8s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @svghadi , its works with user 1000.

kubectl  get po -n kuttl-test-tight-wallaby
NAME                                  READY   STATUS    RESTARTS   AGE
argocd-application-controller-0       1/1     Running   0          13s
argocd-redis-56996db676-nplfr         1/1     Running   0          14s
argocd-repo-server-7f864dd87b-x7r7x   1/1     Running   0          14s
argocd-server-8678799b84-c54dz        1/1     Running   0          14s
kubectl  get po -n kuttl-test-tight-wallaby argocd-redis-56996db676-nplfr -o jsonpath={.spec.securityContext.runAsUser}
1000

Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

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

LGTM

@iam-veeramalla
Copy link
Contributor

LGTM, Thanks @Mangaal

@iam-veeramalla iam-veeramalla merged commit 0ffa69e into argoproj-labs:master Feb 20, 2025
7 checks passed
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.

4 participants