From 8c5a2ec8f79c63767e866540a09b6694f707e0e6 Mon Sep 17 00:00:00 2001 From: ishitasequeira Date: Wed, 26 Jan 2022 23:45:00 -0500 Subject: [PATCH 01/10] proposal: support multiple sources for an Application Signed-off-by: ishitasequeira --- .../multiple-sources-for-applications.md | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 docs/proposals/multiple-sources-for-applications.md diff --git a/docs/proposals/multiple-sources-for-applications.md b/docs/proposals/multiple-sources-for-applications.md new file mode 100644 index 0000000000000..e71e7b212faa1 --- /dev/null +++ b/docs/proposals/multiple-sources-for-applications.md @@ -0,0 +1,225 @@ +--- +title: Neat-enhancement-idea +authors: + - "@ishitasequeira" # Authors' github accounts here. +sponsors: + - TBD # List all intereste parties here. +reviewers: + - "@jannfis" + - TBD +approvers: + - "@jannfis" + - TBD + +creation-date: 2022-01-28 +last-updated: 2022-01-28 +--- + +# Multiple Sources for application + +Support more than one source for creating an Application. + +Related Issues: +* https://github.com/argoproj/argo-cd/issues/677 +* https://github.com/argoproj/argo-cd/issues/2789 + +## Open Questions +* Adding external sources to the Application reource would add additional latencies for creation/reconcilation process. Should we add it to the doc in Risks? +* + +## Summary + +Currently, Argo CD supports Applications with only a single ApplicationSource. In certain scenarios, it would be useful to support more than one source for the application. For example, consider a need to create multiple applications with the same manifests. Today we would have to copy manifest files from one application to another. + +Creating applications from multiple sources would allow users to configure multiple services stored at various sources within the same application. + +## Motivation + +The main motivation behind this enhancement proposal is to allow users to create an application using services that are stored in various sources. + +### Goals + +The goals of the enhancement are: + +#### **Supporting multiple sources for creating an application** + +Users should be able to specify multiple sources to add services to the application. Argo CD should compile all the sources and reconcile each source individually for creating the application. + +#### **Allow specifying external value files for Helm repositories** + +Users should be able to specify different sources for Helm charts and values files. The Helm charts specified by the user could be available in Git or Helm repository and the value files are stored in Git. Argo CD should track changes in both the Helm charts and the value files repository and reconcile the application. + +#### Changes to UI + +The UI should allow users to add multiple sources while creating the application. For each source, UI should allow users to add multiple external values files Helm projects. + +#### Changes to cli + +The cli would need to support adding a list of resources instead of just one while creating the application. Also, just like `--values` field for adding values files, we would need an option to allow external value files to the application. + +### Non-goals +* Allow reconciliation from Argo CD Application resources that are located at various sources. We believe this would be possible with some adaptions in the reconcilation workflow. + +## Proposal + +### Add new `sources` field in Application Spec + +The proposal is to add a new field `sources` which would allow users to input list of `ApplicationSource`s. We would mark field `source` as deprecated and would override the details under `source` with details under `sources` field. + +```yaml +spec: + source: + repoURL: https://charts.bitnami.com/bitnami + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + chart: mysql + sources: # new field + - repoURL: https://charts.bitnami.com/bitnami + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + chart: mysql +``` + +### Add `externalValues` field in helm section of Application Spec + +Along with new `sources` field, add a new field for accepting external value files `externalValueFiles` under the `helm` section of each ApplicationSource. + +```yaml +sources: + - repoURL: https://charts.bitnami.com/bitnami + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + externalValueFiles: # new field + - repoURL: https://github.com/KaiReichart/argo-test-values.git + targetRevision: main + valueFiles: + - values.yaml + chart: mysql +``` + +### Combined Example Application yaml + +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: grafana + namespace: argocd +spec: + destination: + namespace: monitoring + server: https://some.k8s.url.com:6443 + project: default + source: + repoURL: https://charts.bitnami.com/bitnami + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + chart: mysql + sources: # new field + - repoURL: https://charts.bitnami.com/bitnami + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + externalValueFiles: # new field + - repoURL: https://github.com/KaiReichart/argo-test-values.git + targetRevision: main + valueFiles: + - values.yaml + chart: mysql + - repoURL: https://charts.bitnami.com/bitnami1 + targetRevision: 8.5.8 + helm: + valueFiles: + - values.yaml + externalValueFiles: + - repoURL: https://github.com/KaiReichart/argo-test-values.git + targetRevision: main + valueFiles: + - values.yaml + chart: mysql + syncPolicy: + automated: {} +``` + +### Use cases + +Add a list of detailed use cases this enhancement intends to take care of. + +#### Use case 1: +As a user, I have an Application that uses the [elasticsearch](https://github.com/helm/charts/tree/master/incubator/elasticsearch) helm chart as source. Today, user needs to create a separate Application to configure the [elasticsearch-exporter](https://github.com/helm/charts/tree/master/stable/elasticsearch-exporter +) to expose Prometheus metrics. +https://github.com/argoproj/argo-cd/issues/677 + +#### Use case 2: +We have a Helm Chart which is used in 30+ Services and each of them is customized for 3 possible environments. +Replicating this Chart 30 times without a centralized Repo looks dirty. Can be a show stopper for the whole migration. +Modifying the Applica`tion definition is not an option since the whole goal is to reduce the rights that the CI-solution has. Giving it the right to update all Application-definitions from various teams in the argocd namespace is a a hard thing to convince people with. + + +### Implementation Details + +#### Attach multiple sources to the Application + +To allow multiple sources to the Application, we would add a new field `sources` which would allow users to input list of `ApplicationSource`s. As part of this update and to support backward compatibilty, we would mark field `source` as deprecated and remove it in later revisions. + +To avoid complexity on the deciding the list of sources, if both `source` and `sources` fields are specified, we would override the source under `source` field with all the sources under `sources` field. + +#### Invalidating existing cache + +Argo CD benefits from the assumption of a single repo per application to detect changes and to cache the results. But this enhancement now requires us to look at all the source repo "HEAD"s and invalidate the cache if any one of them changes. + +#### Reconcilation of the Application + +As we would have multiple sources as part of the same Application, we would need to track updates to each source and reconcile the Application for each source. When one of the sources change, we would need to ensure that target revisions of other sources are up-to-date, e.g. force a simple refresh to see if target revision of the source (e.g. HEAD), still points to revisionX for example. + +#### Updates to UI +Today, we allow users to add only one source in the UI. We would need to update the UI to add multiple sources and configure specific + +#### Updates to cli + +We would need to create new options to the `argocd app create` command for adding multiple sources to the Application. + +For supporting external Helm value files, we would need to introduce a new option similar to existing `--values` option for Helm projects to support external values files. This options would need to be added individually to each source. + + +### Security Considerations + +Good unit- and end-to-end tests need to be in place for this functionality to ensure we don't accidentally introduce a change that would allow any form of uncontrolled association between an Application and its resources. + +### Risks and Mitigations + +#### Uncontrolled number of sources added to Application + +The users might add a huge number of external sources to the Application, with a potential performance impact on the Argo CD creation/reconcilation processes. This would apply even for the external value files for Helm projects. + +A possible mitigation to this would be to enforce the number of external sources allowed per Application. + +#### Unauthorised access to external resources + +The users might reference the source that has not been whitelisted in the project. This might lead to access issues and failure to sync the Application. + +A possible solution would be to check for the source repository to be whitelisted in the project before syncing and report appropriate error messages in respective scenarios. + + +### Upgrade / Downgrade Strategy + +Upgrading to a version implementing this proposal should be frictionless and wouldn't require administrators to perform any changes in the configuration to keep the current behaviour. Application created without the new field `.spec.sources` being set will keep their behaviour, since they will allow Application resources to be created the same way they are today. + +Downgrading would not be easily possible once users start to make use of the feature and create Applications with the new field `.spec.sources` being set. The Application would no longer be able to recognize the resources and will fail the reconcilation/creation step. + + +## Drawbacks + +* Downgrade/rollback would not be easily possible + +## Alternatives + From f76e23e9e74df213556c9701b31578691f1d812d Mon Sep 17 00:00:00 2001 From: ishitasequeira Date: Sun, 6 Feb 2022 15:41:05 -0500 Subject: [PATCH 02/10] addressed PR comments Signed-off-by: ishitasequeira --- .../multiple-sources-for-applications.md | 88 ++++++++++--------- 1 file changed, 47 insertions(+), 41 deletions(-) diff --git a/docs/proposals/multiple-sources-for-applications.md b/docs/proposals/multiple-sources-for-applications.md index e71e7b212faa1..73441413cab9f 100644 --- a/docs/proposals/multiple-sources-for-applications.md +++ b/docs/proposals/multiple-sources-for-applications.md @@ -6,7 +6,7 @@ sponsors: - TBD # List all intereste parties here. reviewers: - "@jannfis" - - TBD + - "@crenshaw-dev" approvers: - "@jannfis" - TBD @@ -20,12 +20,11 @@ last-updated: 2022-01-28 Support more than one source for creating an Application. Related Issues: -* https://github.com/argoproj/argo-cd/issues/677 -* https://github.com/argoproj/argo-cd/issues/2789 +* [Proposal: Support multiple sources for an application](https://github.com/argoproj/argo-cd/issues/677) +* [Helm chart + values from Git](https://github.com/argoproj/argo-cd/issues/2789) ## Open Questions * Adding external sources to the Application reource would add additional latencies for creation/reconcilation process. Should we add it to the doc in Risks? -* ## Summary @@ -51,11 +50,11 @@ Users should be able to specify different sources for Helm charts and values fil #### Changes to UI -The UI should allow users to add multiple sources while creating the application. For each source, UI should allow users to add multiple external values files Helm projects. +The UI should allow users to add multiple sources while creating the application. For each source, UI should allow users to add multiple external values files Helm projects. We would need a separate proposal for changes to UI. #### Changes to cli -The cli would need to support adding a list of resources instead of just one while creating the application. Also, just like `--values` field for adding values files, we would need an option to allow external value files to the application. +The cli would need to support adding a list of resources instead of just one while creating the application. Also, just like `--values` field for adding values files, we would need an option to allow external value files to the application. We would need a separate proposal for changes to cli. ### Non-goals * Allow reconciliation from Argo CD Application resources that are located at various sources. We believe this would be possible with some adaptions in the reconcilation workflow. @@ -64,24 +63,26 @@ The cli would need to support adding a list of resources instead of just one whi ### Add new `sources` field in Application Spec -The proposal is to add a new field `sources` which would allow users to input list of `ApplicationSource`s. We would mark field `source` as deprecated and would override the details under `source` with details under `sources` field. +The proposal is to add a new field `sources` which would allow users to input list of `ApplicationSource`s. We would mark field `source` as deprecated and would ignore the details under `source` with details under `sources` field. + +Below example shows how the yaml would look like for `source` and `sources` field. We would ignore the `source` field and apply the resources mentioned under `sources` field. ```yaml spec: source: - repoURL: https://charts.bitnami.com/bitnami - targetRevision: 8.5.8 + repoURL: https://github.com/elastic/helm-charts/tree/main/elasticsearch + targetRevision: 6.8 helm: - valueFiles: + valueFiles: - values.yaml - chart: mysql + chart: elasticsearch sources: # new field - - repoURL: https://charts.bitnami.com/bitnami - targetRevision: 8.5.8 + - repoURL: https://github.com/helm/charts/tree/master/incubator/elasticsearch + targetRevision: master helm: valueFiles: - values.yaml - chart: mysql + chart: elasticsearch ``` ### Add `externalValues` field in helm section of Application Spec @@ -90,17 +91,17 @@ Along with new `sources` field, add a new field for accepting external value fil ```yaml sources: - - repoURL: https://charts.bitnami.com/bitnami - targetRevision: 8.5.8 - helm: - valueFiles: - - values.yaml - externalValueFiles: # new field - - repoURL: https://github.com/KaiReichart/argo-test-values.git - targetRevision: main - valueFiles: - - values.yaml - chart: mysql + - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch + targetRevision: master + helm: + valueFiles: + - values.yaml + externalValueFiles: # new field + - repoURL: https://github.com/helm/charts.git + targetRevision: main + valueFiles: + - stable/elasticsearch/values.yaml + chart: elasticsearch ``` ### Combined Example Application yaml @@ -117,35 +118,35 @@ spec: server: https://some.k8s.url.com:6443 project: default source: - repoURL: https://charts.bitnami.com/bitnami - targetRevision: 8.5.8 + repoURL: https://github.com/helm/charts/tree/master/incubator/elasticsearch + targetRevision: master helm: valueFiles: - values.yaml - chart: mysql + chart: elasticsearch sources: # new field - - repoURL: https://charts.bitnami.com/bitnami - targetRevision: 8.5.8 + - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch + targetRevision: master helm: valueFiles: - values.yaml externalValueFiles: # new field - - repoURL: https://github.com/KaiReichart/argo-test-values.git - targetRevision: main + - repoURL: https://github.com/helm/charts.git + targetRevision: master valueFiles: - - values.yaml - chart: mysql - - repoURL: https://charts.bitnami.com/bitnami1 - targetRevision: 8.5.8 + - stable/elasticsearch/values.yaml + chart: elasticsearch + - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch-exporter + targetRevision: master helm: valueFiles: - values.yaml externalValueFiles: - - repoURL: https://github.com/KaiReichart/argo-test-values.git + - repoURL: https://github.com/helm/charts.git targetRevision: main valueFiles: - - values.yaml - chart: mysql + - stable/elasticsearch-exporter/values.yaml + chart: elasticsearch-exporter syncPolicy: automated: {} ``` @@ -160,10 +161,12 @@ As a user, I have an Application that uses the [elasticsearch](https://github.co https://github.com/argoproj/argo-cd/issues/677 #### Use case 2: +As per one of the [comment]((https://github.com/argoproj/argo-cd/issues/2789#issuecomment-562495307)) on the issue [Helm chart + values files from Git](https://github.com/argoproj/argo-cd/issues/2789): +``` We have a Helm Chart which is used in 30+ Services and each of them is customized for 3 possible environments. Replicating this Chart 30 times without a centralized Repo looks dirty. Can be a show stopper for the whole migration. -Modifying the Applica`tion definition is not an option since the whole goal is to reduce the rights that the CI-solution has. Giving it the right to update all Application-definitions from various teams in the argocd namespace is a a hard thing to convince people with. - +Modifying the Application definition is not an option since the whole goal is to reduce the rights that the CI-solution has. Giving it the right to update all Application-definitions from various teams in the argocd namespace is a a hard thing to convince people with. +``` ### Implementation Details @@ -173,6 +176,8 @@ To allow multiple sources to the Application, we would add a new field `sources` To avoid complexity on the deciding the list of sources, if both `source` and `sources` fields are specified, we would override the source under `source` field with all the sources under `sources` field. +** Depracating `source` field:** - Once the `sources` field is implemented in UI and cli as well, we will mark the `source` field as deprecated. At the same time, we will log `WARNING` messages and add UI blurbs about the deprecation. When maintainers feel confident about the adoption of the `sources` field, we will remove the `source` field from later releases. + #### Invalidating existing cache Argo CD benefits from the assumption of a single repo per application to detect changes and to cache the results. But this enhancement now requires us to look at all the source repo "HEAD"s and invalidate the cache if any one of them changes. @@ -190,6 +195,7 @@ We would need to create new options to the `argocd app create` command for addin For supporting external Helm value files, we would need to introduce a new option similar to existing `--values` option for Helm projects to support external values files. This options would need to be added individually to each source. +As per the community call on February 3, changes to UI and cli are huge and are not planned to be part of first iteration. ### Security Considerations From 30dabbae440b2bebc0781baa744c4b186e0dffc5 Mon Sep 17 00:00:00 2001 From: ishitasequeira Date: Thu, 17 Mar 2022 23:33:55 -0700 Subject: [PATCH 03/10] update summary Signed-off-by: ishitasequeira --- docs/proposals/multiple-sources-for-applications.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/proposals/multiple-sources-for-applications.md b/docs/proposals/multiple-sources-for-applications.md index 73441413cab9f..2497d17f1720c 100644 --- a/docs/proposals/multiple-sources-for-applications.md +++ b/docs/proposals/multiple-sources-for-applications.md @@ -28,7 +28,12 @@ Related Issues: ## Summary -Currently, Argo CD supports Applications with only a single ApplicationSource. In certain scenarios, it would be useful to support more than one source for the application. For example, consider a need to create multiple applications with the same manifests. Today we would have to copy manifest files from one application to another. +Currently, Argo CD supports Applications with only a single ApplicationSource. In certain scenarios, it would be useful to support more than one source for the application. For example, consider a need to create multiple deployments of the same application and manifests but manifests can come from different sources. Today we would have to copy manifest files from one application to another. + +For example, from [one of the comments on this proposal PR](https://github.com/argoproj/argo-cd/pull/8322/files#r799624767) +``` +An independent support of the Helm charts and their values files. This opens a door to such highly requested scenarios like multiple deployments of the same (possibly external) Helm chart with different values files or an independent migration to a newer Helm chart version for the same applications installed in Test and Production environments. +``` Creating applications from multiple sources would allow users to configure multiple services stored at various sources within the same application. @@ -57,7 +62,7 @@ The UI should allow users to add multiple sources while creating the application The cli would need to support adding a list of resources instead of just one while creating the application. Also, just like `--values` field for adding values files, we would need an option to allow external value files to the application. We would need a separate proposal for changes to cli. ### Non-goals -* Allow reconciliation from Argo CD Application resources that are located at various sources. We believe this would be possible with some adaptions in the reconcilation workflow. +* ## Proposal @@ -227,5 +232,3 @@ Downgrading would not be easily possible once users start to make use of the fea * Downgrade/rollback would not be easily possible -## Alternatives - From 44d6e766652549c7cd4e3f8a175f4e5c1eaf6666 Mon Sep 17 00:00:00 2001 From: pasha-codefresh Date: Tue, 22 Mar 2022 00:54:19 +0200 Subject: [PATCH 04/10] feat: move watch params to struct (#8819) * add to approvers Signed-off-by: pashavictorovich * watch opts move to struct Signed-off-by: pashavictorovich * watch opts move to struct Signed-off-by: pashavictorovich Signed-off-by: ishitasequeira --- cmd/argocd/commands/app_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/argocd/commands/app_test.go b/cmd/argocd/commands/app_test.go index 2a1ecd71c9f04..b19036a779297 100644 --- a/cmd/argocd/commands/app_test.go +++ b/cmd/argocd/commands/app_test.go @@ -13,6 +13,8 @@ import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) From 911a7249f0c6924d19a162f2460fea8d2faa9338 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Tue, 22 Mar 2022 10:57:30 -0700 Subject: [PATCH 05/10] Merge pull request from GHSA-2f5v-8r3f-8pww * fix: application resource APIs must enforce project restrictions Signed-off-by: Alexander Matyushentsev * Fix unit tests Signed-off-by: jannfis Co-authored-by: jannfis Signed-off-by: ishitasequeira --- test/e2e/fixture/project/actions.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/fixture/project/actions.go b/test/e2e/fixture/project/actions.go index 3123da3f43768..7e4cf57f5ca3e 100644 --- a/test/e2e/fixture/project/actions.go +++ b/test/e2e/fixture/project/actions.go @@ -8,6 +8,8 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/test/e2e/fixture" + "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // this implements the "when" part of given/when/then From 172b35f17a2c5b4fd1fd66014ca5357b5e1832e7 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Tue, 22 Mar 2022 15:52:48 -0400 Subject: [PATCH 06/10] chore: fix imports and unit tests (#8857) * chore: fix imports Signed-off-by: Michael Crenshaw * chore: fix unit test Signed-off-by: Michael Crenshaw * chore: keep changes minimal Signed-off-by: Michael Crenshaw * chore: fix another test Signed-off-by: Michael Crenshaw Signed-off-by: ishitasequeira --- test/e2e/fixture/project/actions.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/fixture/project/actions.go b/test/e2e/fixture/project/actions.go index 7e4cf57f5ca3e..1b019e7078082 100644 --- a/test/e2e/fixture/project/actions.go +++ b/test/e2e/fixture/project/actions.go @@ -10,6 +10,9 @@ import ( "github.com/argoproj/argo-cd/v2/test/e2e/fixture" "github.com/stretchr/testify/require" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/argoproj/argo-cd/v2/test/e2e/fixture" ) // this implements the "when" part of given/when/then From c0fb06ef79f96ef971d8a4a9b16ca9a2b800146f Mon Sep 17 00:00:00 2001 From: pasha-codefresh Date: Thu, 24 Mar 2022 17:53:16 +0200 Subject: [PATCH 07/10] feat: operation result and history table tests (#8887) * add to approvers Signed-off-by: pashavictorovich * print tables additional tests Signed-off-by: pashavictorovich * move to %q Signed-off-by: pashavictorovich Signed-off-by: ishitasequeira --- cmd/argocd/commands/app_test.go | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cmd/argocd/commands/app_test.go b/cmd/argocd/commands/app_test.go index b19036a779297..81436232afdc5 100644 --- a/cmd/argocd/commands/app_test.go +++ b/cmd/argocd/commands/app_test.go @@ -640,3 +640,37 @@ func TestPrintApplicationNames(t *testing.T) { t.Fatalf("Incorrect print params output %q, should be %q", output, expectation) } } + +func TestPrintApplicationHistoryTable(t *testing.T) { + histories := []v1alpha1.RevisionHistory{ + { + ID: 1, + Source: v1alpha1.ApplicationSource{ + TargetRevision: "1", + }, + }, + { + ID: 2, + Source: v1alpha1.ApplicationSource{ + TargetRevision: "2", + }, + }, + { + ID: 3, + Source: v1alpha1.ApplicationSource{ + TargetRevision: "3", + }, + }, + } + + output, _ := captureOutput(func() error { + printApplicationHistoryTable(histories) + return nil + }) + + expectation := "ID DATE REVISION\n1 0001-01-01 00:00:00 +0000 UTC 1\n2 0001-01-01 00:00:00 +0000 UTC 2\n3 0001-01-01 00:00:00 +0000 UTC 3\n" + + if output != expectation { + t.Fatalf("Incorrect print operation output %q, should be %q", output, expectation) + } +} From 30d55c019a3c15f84c19c23a0dbe079e440fe11a Mon Sep 17 00:00:00 2001 From: rishabh625 <43094970+rishabh625@users.noreply.github.com> Date: Thu, 31 Mar 2022 19:10:41 +0530 Subject: [PATCH 08/10] fix: corrected applicationset binary name in manifests (#8954) Signed-off-by: rishabh625 Signed-off-by: ishitasequeira From b98c1290970369e984db0eba6e393aa320dc4da7 Mon Sep 17 00:00:00 2001 From: ishitasequeira Date: Fri, 1 Apr 2022 12:03:05 -0400 Subject: [PATCH 09/10] Update proposed design to add 'ref' field instead of externalValuesField Signed-off-by: ishitasequeira --- .../multiple-sources-for-applications.md | 103 ++++++++++-------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/docs/proposals/multiple-sources-for-applications.md b/docs/proposals/multiple-sources-for-applications.md index 2497d17f1720c..6ec30f51f1b46 100644 --- a/docs/proposals/multiple-sources-for-applications.md +++ b/docs/proposals/multiple-sources-for-applications.md @@ -7,12 +7,14 @@ sponsors: reviewers: - "@jannfis" - "@crenshaw-dev" + - "@alexmt" approvers: - "@jannfis" - - TBD + - "@alexmt" + - "@crenshaw-dev" creation-date: 2022-01-28 -last-updated: 2022-01-28 +last-updated: 2022-04-01 --- # Multiple Sources for application @@ -59,7 +61,7 @@ The UI should allow users to add multiple sources while creating the application #### Changes to cli -The cli would need to support adding a list of resources instead of just one while creating the application. Also, just like `--values` field for adding values files, we would need an option to allow external value files to the application. We would need a separate proposal for changes to cli. +The cli would need to support adding a list of resources instead of just one while creating the application. `values` field should allow referencing value files from other sources. We would need a separate proposal for changes to cli. ### Non-goals * @@ -80,33 +82,49 @@ spec: helm: valueFiles: - values.yaml - chart: elasticsearch sources: # new field - - repoURL: https://github.com/helm/charts/tree/master/incubator/elasticsearch + - repoURL: https://github.com/helm/charts targetRevision: master + path: incubator/elasticsearch helm: valueFiles: - values.yaml - chart: elasticsearch ``` -### Add `externalValues` field in helm section of Application Spec +### Make `path/chart` field optional -Along with new `sources` field, add a new field for accepting external value files `externalValueFiles` under the `helm` section of each ApplicationSource. +While adding sources to the application, users can decide not to add `path/chart` field in the application yaml. The controller will not generate manifests for the sources that do not have `path/chart` field specified. For example, in the below application spec, controller will generate the manifest for `elasticsearch` source but not for source `my-repo`. + +``` +spec: + sources: + - repoURL: https://github.com/my-org/my-repo # path is missing so no manifests are generated + targetRevision: master + ref: myRepo # repo is available via symlink "my-repo" + - repoURL: https://github.com/helm/charts + targetRevision: master + path: incubator/elasticsearch # path "incubator/elasticsearch" is used to generate manifests + helm: + valueFiles: + - $myRepo/values.yaml # values.yaml is located in source with reference name $myRepo +``` + +### Add optional `ref` field to Application Source + +For making files accessible to other sources, add a new `ref` field to the source. For example, in below ApplicationSpec, we have added `ref: myRepo` to the `my-repo` repository and have used reference `$myRepo` to the `elasticSearch` repository. ```yaml -sources: - - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch - targetRevision: master - helm: - valueFiles: - - values.yaml - externalValueFiles: # new field - - repoURL: https://github.com/helm/charts.git - targetRevision: main - valueFiles: - - stable/elasticsearch/values.yaml - chart: elasticsearch +spec: + sources: + - repoURL: https://github.com/my-org/my-repo # path is missing so no manifests are generated + targetRevision: master + ref: myRepo # repo is available via symlink "myRepo" + - repoURL: https://github.com/helm/charts + targetRevision: master + path: incubator/elasticsearch # path "incubator/elasticsearch" is used to generate manifests + helm: + valueFiles: + - $myRepo/values.yaml # values.yaml is located in source with reference name $myRepo ``` ### Combined Example Application yaml @@ -123,39 +141,35 @@ spec: server: https://some.k8s.url.com:6443 project: default source: - repoURL: https://github.com/helm/charts/tree/master/incubator/elasticsearch + repoURL: https://github.com/helm/charts targetRevision: master helm: valueFiles: - values.yaml - chart: elasticsearch + chart: incubator/elasticsearch sources: # new field - - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch + # application that consists of MongoDB and ElasticSearch resources + - repoURL: https://github.com/helm/charts targetRevision: master - helm: - valueFiles: - - values.yaml - externalValueFiles: # new field - - repoURL: https://github.com/helm/charts.git - targetRevision: master - valueFiles: - - stable/elasticsearch/values.yaml - chart: elasticsearch - - repoURL: https://github.com/helm/charts/tree/master/stable/elasticsearch-exporter + path: incubator/mongodb + - repoURL: https://github.com/helm/charts + targetRevision: master + path: incubator/elasticsearch + - repoURL: https://github.com/my-org/my-repo # path is missing so no manifests are generated targetRevision: master + ref: myRepo # repo is available via symlink "my-repo" + - repoURL: https://github.com/helm/charts + targetRevision: master + path: incubator/elasticsearch # path "incubator/elasticsearch" is used to generate manifests helm: valueFiles: - - values.yaml - externalValueFiles: - - repoURL: https://github.com/helm/charts.git - targetRevision: main - valueFiles: - - stable/elasticsearch-exporter/values.yaml - chart: elasticsearch-exporter + - $myRepo/values.yaml # values.yaml is located in source with reference name $myRepo syncPolicy: automated: {} ``` +In scenarios, where you have same resource mentioned multiple times in the application yaml, the last resource in the source list will override the previous resources. + ### Use cases Add a list of detailed use cases this enhancement intends to take care of. @@ -181,7 +195,7 @@ To allow multiple sources to the Application, we would add a new field `sources` To avoid complexity on the deciding the list of sources, if both `source` and `sources` fields are specified, we would override the source under `source` field with all the sources under `sources` field. -** Depracating `source` field:** - Once the `sources` field is implemented in UI and cli as well, we will mark the `source` field as deprecated. At the same time, we will log `WARNING` messages and add UI blurbs about the deprecation. When maintainers feel confident about the adoption of the `sources` field, we will remove the `source` field from later releases. +**Depracating `source` field:** - Once the `sources` field is implemented in UI and cli as well, we will mark the `source` field as deprecated. At the same time, we will log `WARNING` messages and add UI blurbs about the deprecation. When maintainers feel confident about the adoption of the `sources` field, we will remove the `source` field from later releases. #### Invalidating existing cache @@ -196,15 +210,13 @@ Today, we allow users to add only one source in the UI. We would need to update #### Updates to cli -We would need to create new options to the `argocd app create` command for adding multiple sources to the Application. - -For supporting external Helm value files, we would need to introduce a new option similar to existing `--values` option for Helm projects to support external values files. This options would need to be added individually to each source. +We would need to create new options to the `argocd app create` command for adding multiple sources to the Application. We would also need to introduce allowing `ref` field for sources and to reference the files from symlinked source. As per the community call on February 3, changes to UI and cli are huge and are not planned to be part of first iteration. ### Security Considerations -Good unit- and end-to-end tests need to be in place for this functionality to ensure we don't accidentally introduce a change that would allow any form of uncontrolled association between an Application and its resources. +Good unit- and end-to-end tests need to be in place for this functionality to ensure we don't accidentally introduce a change that would break the feature. ### Risks and Mitigations @@ -231,4 +243,3 @@ Downgrading would not be easily possible once users start to make use of the fea ## Drawbacks * Downgrade/rollback would not be easily possible - From 70431e93194ca4e8179a58ec12c9c9eeec655e2c Mon Sep 17 00:00:00 2001 From: ishitasequeira Date: Fri, 1 Apr 2022 12:39:20 -0400 Subject: [PATCH 10/10] remove unwanted changes from PR added while rebase Signed-off-by: ishitasequeira --- cmd/argocd/commands/app_test.go | 36 ----------------------------- test/e2e/fixture/project/actions.go | 5 ---- 2 files changed, 41 deletions(-) diff --git a/cmd/argocd/commands/app_test.go b/cmd/argocd/commands/app_test.go index 81436232afdc5..2a1ecd71c9f04 100644 --- a/cmd/argocd/commands/app_test.go +++ b/cmd/argocd/commands/app_test.go @@ -13,8 +13,6 @@ import ( "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/assert" - "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) @@ -640,37 +638,3 @@ func TestPrintApplicationNames(t *testing.T) { t.Fatalf("Incorrect print params output %q, should be %q", output, expectation) } } - -func TestPrintApplicationHistoryTable(t *testing.T) { - histories := []v1alpha1.RevisionHistory{ - { - ID: 1, - Source: v1alpha1.ApplicationSource{ - TargetRevision: "1", - }, - }, - { - ID: 2, - Source: v1alpha1.ApplicationSource{ - TargetRevision: "2", - }, - }, - { - ID: 3, - Source: v1alpha1.ApplicationSource{ - TargetRevision: "3", - }, - }, - } - - output, _ := captureOutput(func() error { - printApplicationHistoryTable(histories) - return nil - }) - - expectation := "ID DATE REVISION\n1 0001-01-01 00:00:00 +0000 UTC 1\n2 0001-01-01 00:00:00 +0000 UTC 2\n3 0001-01-01 00:00:00 +0000 UTC 3\n" - - if output != expectation { - t.Fatalf("Incorrect print operation output %q, should be %q", output, expectation) - } -} diff --git a/test/e2e/fixture/project/actions.go b/test/e2e/fixture/project/actions.go index 1b019e7078082..3123da3f43768 100644 --- a/test/e2e/fixture/project/actions.go +++ b/test/e2e/fixture/project/actions.go @@ -6,11 +6,6 @@ import ( "github.com/stretchr/testify/require" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/test/e2e/fixture" - "github.com/stretchr/testify/require" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/test/e2e/fixture" )