feat: Deploy ArgoCD on cluster with security context (#3060)

[Leletir]

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Optional. My organization is added to the README.
• I've signed the CLA and my build is green (troubleshooting builds).

[Leletir]

@jannfis do you know why the version of Kustomize is 2.0.3 ?
In order to avoid code duplication I'm using kustomize build --load_restrictor LoadRestrictionsNone to be able to access patches from another directory when I'm building ha but it's not available on kustomize 2.x

[codecov]

Codecov Report

Merging #3097 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #3097   +/-   ##
======================================
  Coverage    38.3%   38.3%           
======================================
  Files         168     168           
  Lines       18206   18206           
  Branches      272     272           
======================================
  Hits         6974    6974           
  Misses      10358   10358           
  Partials      874     874

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d528629...064d61b. Read the comment docs.

[Leletir]

Hello @jessesuen and @jannfis, I'm looking for a way to not duplicate patches between manifests/security-context/base/overlays and manifests/ha/security-context/base/overlays, do you have any ideas ?

Thank you for your help

[jannfis]

@jannfis do you know why the version of Kustomize is 2.0.3 ?
In order to avoid code duplication I'm using kustomize build --load_restrictor LoadRestrictionsNone to be able to access patches from another directory when I'm building ha but it's not available on kustomize 2.x

Hi @Leletir, hm, indeed I think it was just forgotten to update Kustomize version in the build tools image to the version that is also included in recent versions of ArgoCD (v3.2.1 currently). I have just submitted #3099 to bring it up-to-date, I'm not sure whether there is a reason behind it that I do not know of :)

However, please restrain from modifying --load_restrictor in calls to kustomize, since the Kustomize sources should be compatible with ArgoCD (i.e. manifests should be buildable when using the Kustomize source as an ArgoCD application).

[Leletir]

Hi @jannfis, thank you for your PR !

I completely agree with you on this point, but I have no idea how to not duplicate the patches between ha and security-context, maybe using multiple-bases with kustomize ... if you have any idea, I'm more than interested !

[alexmt]

@jannfis do you know why the version of Kustomize is 2.0.3 ?
In order to avoid code duplication I'm using kustomize build --load_restrictor LoadRestrictionsNone to be able to access patches from another directory when I'm building ha but it's not available on kustomize 2.x

@Leletir we are still using kustomize 2 because of some users install argocd using https://github.com/replicatedhq/ship or kubectl apply -k. Need to check if these tools aready migrated to kustomize 3

[Leletir]

Hello @alexmt, you're right kubectl 1.16 is still using kustomize 2.0.3.

So do you have any idea how I can avoid to duplicate the patches between ha et security context?

[jannfis]

Hi @Leletir, sorry for the late reply. Unfortunately, I don't know of an elegant or easy way to deduplicate the Kustomize overlays in the current scenario. I think the way you've done is what's possible with 2.0.x, so I'd be fine with this change.

We can (and should) change the overlays once we can incorporate the 3.x branch of Kustomize.

How does that sound to you?

[Leletir]

I agree with you on this point.
So what are the next steps ?

[jannfis]

@jannfis do you know why the version of Kustomize is 2.0.3 ?
In order to avoid code duplication I'm using kustomize build --load_restrictor LoadRestrictionsNone to be able to access patches from another directory when I'm building ha but it's not available on kustomize 2.x

@Leletir we are still using kustomize 2 because of some users install argocd using https://github.com/replicatedhq/ship or kubectl apply -k. Need to check if these tools aready migrated to kustomize 3

Hm, I checked ship and this project does not seem to be actively developed anymore in favour of kots - looking at their Issue tracker, there are a few requests to support Kustomize v2.10, those are from August last year. So I doubt they will upgrade any time soon.

As for integration into kubectl, sigh. I found some docs at https://github.com/kubernetes-sigs/kustomize#kubectl-integration and it doesn't sound too promising on upgrading their kustomize integration either.

[jannfis]

@alexmt do you have any objections to add a third set of installation manifests as in this PR? If not, I'll merge this change. I think it's something many people would want to have working out-of-the-box.

[jannfis]

Hi @Leletir, sorry for the delay with this PR. Can you please merge latest upstream master into your branch and update this PR, because we recently merged #3147 that touched quite a lot of the HA manifests - otherwise merging this change would break, I think (or at least leave us with a broken security enabled HA manifest).

[Leletir]

Hello @jannfis, no problem, I'll have a look this weekend.

[jessesuen]

Instead of:

      securityContext:
        runAsUser: 999
        runAsGroup: 999
        fsGroup: 999 

I think https://github.com/argoproj/argo-cd/pull/3108/files should be merged and the security context should instead be:

      securityContext:
        runAsNonRoot: true

This has the advantage of not coordinating the userid/groupid to the Dockerfile.

[jessesuen]

See above comment. I think this is preferred:

      securityContext:
        runAsNonRoot: true

[jannfis]

Agreed, @jessesuen - actually makes sense.

[Leletir]

Totally agree on this one

[jessesuen]

@Leletir now that #3108 is merged, did you want to apply the requested changes?

[sbose78]

@jannfis @alexmt This can be closed .

This PR gets superseded by:

#5786
#5819
#5820