feat: support custom helm values file schemes #8535

alexmt · 2022-02-16T20:39:24Z

Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com

It is not secure to allow any url scheme for a remote helm values file. The PR introduces are new setting in argocd-cm ConfigMap that allows user to configure allowed schemes:

  # Comma delimited list of additional custom remote values file schemes (http are https are allowed by default)
  helm.valuesFileSchemes: s3, git

codecov · 2022-02-16T21:03:04Z

Codecov Report

Merging #8535 (293131f) into master (1faa9b0) will increase coverage by 0.10%.
The diff coverage is 81.57%.

@@            Coverage Diff             @@
##           master    #8535      +/-   ##
==========================================
+ Coverage   42.33%   42.44%   +0.10%     
==========================================
  Files         176      176              
  Lines       22860    22889      +29     
==========================================
+ Hits         9678     9715      +37     
+ Misses      11799    11789      -10     
- Partials     1383     1385       +2

Impacted Files	Coverage Δ
pkg/apis/application/v1alpha1/types.go	`55.08% <ø> (ø)`
controller/state.go	`73.90% <50.00%> (-0.27%)`	⬇️
server/application/application.go	`32.27% <66.66%> (+1.00%)`	⬆️
util/settings/settings.go	`48.14% <80.00%> (+0.60%)`	⬆️
util/argo/argo.go	`64.01% <88.23%> (+1.00%)`	⬆️
reposerver/repository/repository.go	`58.66% <100.00%> (+0.12%)`	⬆️

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 1faa9b0...293131f. Read the comment docs.

jannfis · 2022-02-16T21:10:55Z

If we make this configurable, should we also allow to forbid http and https (e.g. when helm.valuesFileSchemes is set, do not automatically pull in http and https schemes)?

jkroepke · 2022-02-16T21:27:24Z

I would agree with jannfis. Remove the hard-coded values. If http, https should allowed, they should be included in helm.valuesFileSchemes. The default value of helm.valuesFileSchemes could be http,https. But if helm.valuesFileSchemes= is defined (empty value), no remote protocols are allowed.

alexmt · 2022-02-16T21:35:31Z

@jannfis , @jkroepke I agree. Did not think about it. But http and https should be enabled by default, right?

jannfis · 2022-02-16T21:43:10Z

Yeah, probably keep http and https as the defaults for now. I was also wondering (apart from this change), whether we should introduce an URL allow list for this feature.

terrytangyuan · 2022-02-16T21:50:16Z

docs/operator-manual/argocd-cm.yaml

@@ -216,6 +216,9 @@ data:
  kustomize.version.v3.5.1: /custom-tools/kustomize_3_5_1
  kustomize.version.v3.5.4: /custom-tools/kustomize_3_5_4

+  # Comma delimited list of additional custom remote values file schemes (http are https are allowed by default)
+  helm.valuesFileSchemes: s3, git


Would helm.valuesFileSchemesEnabled or helm.valuesFileSchemesAllowed be better?

For some reason, I like just helm.valuesFileSchemes. Boh enabled and allowed looks a little redundant to me because I cannot imagine any other option.

I might be wrong and don't have a strong opinion about it, to be honest. @terrytangyuan let me know please if you really think that some clarification is required and we should use helm.valuesFileSchemesEnabled/helm.valuesFileSchemesAllowed.

Sounds good to me. No need to change.

alexmt · 2022-02-17T00:29:31Z

@jannfis , @jkroepke implemented suggested change that allows disabling http/https.

Regarding URL allow list, I agree. This change is just to restore the ability to use value files and help plugins. In v2.4 or later we can enhance it and support configuring URLs

PTAL

jessesuen

LGTM

jkroepke

👍

jannfis

LGTM

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

jkroepke · 2022-02-17T22:18:31Z

Hi, is there a chance to backport this into the 2.2.x branch? In case if not, is there any release of ArgoCD 2.3 planned?

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

stephaneetje · 2022-02-28T09:11:47Z

Hello, is it resolved ? i see a merge but i added helm.valuesFileSchemes: s3, git, secrets to my argocd-cm but i still get rpc error: code = Unknown desc = Manifest generation error (cached): the URL scheme 'secrets' is not allowed.
Am i doing something wrong ? I could not find documentation

stephaneetje · 2022-03-02T08:40:56Z

Anyone that successfully added the option ?

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

jmclean-starburst · 2022-03-04T18:40:29Z

Hello, is it resolved ? i see a merge but i added helm.valuesFileSchemes: s3, git, secrets to my argocd-cm but i still get rpc error: code = Unknown desc = Manifest generation error (cached): the URL scheme 'secrets' is not allowed. Am i doing something wrong ? I could not find documentation

Im still having this issue, using v2.3.0-rc5; i have verified my argocd-cm contains the below

> kgcm argocd-cm -o yaml | grep valuesFileSchemes | grep -v apiVersion
  helm.valuesFileSchemes: http, https, secrets

@jkroepke - I have followed your guidance in https://github.com/jkroepke/helm-secrets/blob/88609fb95f79720f64268c317ef079dac90f75f3/docs/ArgoCD%20Integration.md

jmclean-starburst · 2022-03-04T18:43:58Z

silly me, i noticed helm.valuesFileSchemes is not in https://github.com/argoproj/argo-cd/blob/v2.3.0-rc5/docs/operator-manual/argocd-cm.yaml

petr4 · 2022-03-23T23:45:56Z

Hey there. is it merged? the schema options still does not work for 2.3.2.

rgeraskin · 2022-03-25T16:28:42Z

@petr4 did you try to hard refresh your app? looks like it's cached error

petr4 · 2022-03-28T18:29:38Z

@rgeraskin Hy there, it works as well. In my case the issue was in wrong argocd service to apply configs. And yes - refresh apps definitely helps with correct works

amohamedhey · 2022-04-19T15:52:31Z

@rgeraskin @petr4
I have tried this
helm.valuesFileSchemes: http, https
but it didn't work. should I restart any pod after update the configmap?

petr4 · 2022-04-20T02:32:13Z

@amohamedhey Hey! Any changes should be reread from cm to pod, i guess restart can help, in my case i just have added helm {{ data }} (please, read helm templates functions) to annotation

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>

MeNsaaH · 2022-05-30T21:37:37Z

@alexmt

I have upgraded. However, I noticed all my argocd app diffs command are broken with the same error

the URL scheme 'secrets' is not allowed

Is there something, I'm missing? does the argocd app diff ignore the argocd-cm?

#9200

arthurk · 2022-06-29T04:13:39Z

This PR has introduced a bug with the argocd app diff command when the --local flag is passed. The settings will not be read and the diff command will show the URL scheme 'secrets' is not allowed errors

myrondev · 2022-11-15T10:53:39Z

Hi. Did anyone solve this issue?

myrondev · 2022-11-15T10:54:35Z

@rgeraskin @petr4 I have tried this helm.valuesFileSchemes: http, https but it didn't work. should I restart any pod after update the configmap?

It's a good idea to restart pods.

myrondev · 2022-11-15T10:59:53Z

@rgeraskin Hy there, it works as well. In my case the issue was in wrong argocd service to apply configs. And yes - refresh apps definitely helps with correct works

Did you fix it with this? helm.valuesFileSchemes: http, https, secrets

alexmt requested a review from jannfis February 16, 2022 20:39

alexmt added this to the v2.3 milestone Feb 16, 2022

jkroepke mentioned this pull request Feb 16, 2022

Age encryption in ArgoCD is not working: the URL scheme 'secrets+age-import' is not allowed jkroepke/helm-secrets#185

Closed

terrytangyuan reviewed Feb 16, 2022

View reviewed changes

jessesuen approved these changes Feb 17, 2022

View reviewed changes

alexmt force-pushed the 8397-helm-schemas branch from 7d185a5 to e24b887 Compare February 17, 2022 01:18

jkroepke approved these changes Feb 17, 2022

View reviewed changes

jannfis approved these changes Feb 17, 2022

View reviewed changes

feat: support custom helm values file schemes

293131f

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the 8397-helm-schemas branch from e24b887 to 293131f Compare February 17, 2022 18:39

alexmt merged commit 902b6aa into argoproj:master Feb 17, 2022

alexmt deleted the 8397-helm-schemas branch February 17, 2022 19:51

gdsoumya pushed a commit to gdsoumya/argo-cd that referenced this pull request Feb 23, 2022

feat: support custom helm values file schemes (argoproj#8535)

3b6c498

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt added a commit that referenced this pull request Feb 25, 2022

feat: support custom helm values file schemes (#8535)

10b4c47

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt added a commit that referenced this pull request Mar 3, 2022

feat: support custom helm values file schemes (#8535)

a9e1040

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt added a commit that referenced this pull request Mar 4, 2022

feat: support custom helm values file schemes (#8535)

0ef556e

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

romulus-ai mentioned this pull request Mar 7, 2022

Add "helm.valuesFileSchemes" for argocd-cm for usage of helm-secrets argoproj-labs/argocd-operator#583

Open

wojtekidd pushed a commit to wojtekidd/argo-cd that referenced this pull request Apr 25, 2022

feat: support custom helm values file schemes (argoproj#8535)

5d3cdcc

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support custom helm values file schemes #8535

feat: support custom helm values file schemes #8535

alexmt commented Feb 16, 2022

codecov bot commented Feb 16, 2022 •

edited

jannfis commented Feb 16, 2022

jkroepke commented Feb 16, 2022

alexmt commented Feb 16, 2022

jannfis commented Feb 16, 2022

terrytangyuan Feb 16, 2022

alexmt Feb 17, 2022

terrytangyuan Feb 17, 2022

alexmt commented Feb 17, 2022

jessesuen left a comment

jkroepke left a comment

jannfis left a comment

jkroepke commented Feb 17, 2022

stephaneetje commented Feb 28, 2022

stephaneetje commented Mar 2, 2022

jmclean-starburst commented Mar 4, 2022

jmclean-starburst commented Mar 4, 2022

petr4 commented Mar 23, 2022

rgeraskin commented Mar 25, 2022

petr4 commented Mar 28, 2022

amohamedhey commented Apr 19, 2022

petr4 commented Apr 20, 2022

MeNsaaH commented May 30, 2022

arthurk commented Jun 29, 2022

myrondev commented Nov 15, 2022

myrondev commented Nov 15, 2022

myrondev commented Nov 15, 2022

feat: support custom helm values file schemes #8535

feat: support custom helm values file schemes #8535

Conversation

alexmt commented Feb 16, 2022

codecov bot commented Feb 16, 2022 • edited

Codecov Report

jannfis commented Feb 16, 2022

jkroepke commented Feb 16, 2022

alexmt commented Feb 16, 2022

jannfis commented Feb 16, 2022

terrytangyuan Feb 16, 2022

Choose a reason for hiding this comment

alexmt Feb 17, 2022

Choose a reason for hiding this comment

terrytangyuan Feb 17, 2022

Choose a reason for hiding this comment

alexmt commented Feb 17, 2022

jessesuen left a comment

Choose a reason for hiding this comment

jkroepke left a comment

Choose a reason for hiding this comment

jannfis left a comment

Choose a reason for hiding this comment

jkroepke commented Feb 17, 2022

stephaneetje commented Feb 28, 2022

stephaneetje commented Mar 2, 2022

jmclean-starburst commented Mar 4, 2022

jmclean-starburst commented Mar 4, 2022

petr4 commented Mar 23, 2022

rgeraskin commented Mar 25, 2022

petr4 commented Mar 28, 2022

amohamedhey commented Apr 19, 2022

petr4 commented Apr 20, 2022

MeNsaaH commented May 30, 2022

arthurk commented Jun 29, 2022

myrondev commented Nov 15, 2022

myrondev commented Nov 15, 2022

myrondev commented Nov 15, 2022

codecov bot commented Feb 16, 2022 •

edited