Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD #1875

dgershman · 2023-07-02T16:31:59Z

Description

This adds support to have the Zarf Agent mutate the repoURL field inside of the Application CRD of ArgoCD. This would enable those who are utilizing Argo as part of their GitOps deployment strategy to gain the same benefits as the current support that the Agent has present for Flux.

Related Issue

Slack Discussion

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Other (security config, docs update, etc)

Checklist before merging

Test, docs, adr added or updated as needed
Contributor Guide Steps followed

netlify · 2023-07-02T16:32:04Z

✅ Deploy Preview for zarf-docs canceled.

Name	Link
🔨 Latest commit	`11a0d7d`
🔍 Latest deploy log	https://app.netlify.com/sites/zarf-docs/deploys/64da80b9982c880008ae1b61

dgershman · 2023-07-02T16:54:48Z

I have to make another change here as ArgoCD also requires Repository configuration that matches which also defines the authentication. This is actually stored as a Secret.

dgershman · 2023-07-02T18:50:32Z

I have to make another change here as ArgoCD also requires Repository configuration that matches which also defines the authentication. This is actually stored as a Secret.

Ok added this. Running some more tests.

dgershman · 2023-07-02T19:20:16Z

I have to make another change here as ArgoCD also requires Repository configuration that matches which also defines the authentication. This is actually stored as a Secret.

Ok added this. Running some more tests.

Testing looks good. This is ready for review.

Racer159

Overall the code looks good! - just a few small suggested changes.

Also you mentioned you had some tests setup, can you add a test under src/test/e2e/22_git_and_flux_test.go for ArgoCD? (potentially renaming that test file to 22_git_and_gitops_test.go)

Also as a note, we have been standardizing around podinfo for most tests to keep things fast / lean (since we only have to pull / push a single image like podinfo:6.4.0 once) but if that is too far out from what you have we can pull in other simple examples for now as well (like guestbook). If you want you could put the package as an example under examples (if you want to add a README.md to it for Argo) or under src/test/packages with a prefix of 22 to just tie it to that test.

src/internal/agent/http/server.go

src/internal/agent/hooks/argocd-application.go

packages/zarf-agent/manifests/webhook.yaml

dgershman · 2023-07-10T00:21:22Z

While attempting to build a package for argo-cd I'm getting an error because the chart contains a subchart that is not already present in the charts/ path. Is there a way to tell the charts component to run helm dependency update to ensure the subcharts are included?

dgershman · 2023-07-10T00:22:33Z

While attempting to build a package for argo-cd I'm getting an error because the chart contains a subchart that is not already present in the charts/ path. Is there a way to tell the charts component to run helm dependency update to ensure the subcharts are included?

Here is the specific dependency https://github.com/argoproj/argo-helm/blob/main/charts/argo-cd/Chart.yaml#L20.

Racer159 · 2023-07-10T18:27:16Z

While attempting to build a package for argo-cd I'm getting an error because the chart contains a subchart that is not already present in the charts/ path. Is there a way to tell the charts component to run helm dependency update to ensure the subcharts are included?

Here is the specific dependency https://github.com/argoproj/argo-helm/blob/main/charts/argo-cd/Chart.yaml#L20.

Not (yet) though that functionality is about to get in via: #1892 (https://github.com/defenseunicorns/zarf/pull/1892/files#diff-34fa222e42b012c182371020184228343caa160e704cc384d7a96147e59c88faR186)

Racer159 · 2023-07-12T21:31:17Z

With that merge ^ there is now support for building dependencies (which is to update as npm ci is to npm install)

dgershman · 2023-07-13T03:27:44Z

With that merge ^ there is now support for building dependencies (which is to update as npm ci is to npm install)

I have a workaround for this by not using the Git repository and using the Helm repo instead. However, it looks like it's failing before it can have a chance to install dependencies with the Git Repository method. It's not a blocker for me, but something to perhaps be aware of.

 WARNING  Helm is unable to save the archive and create the package
  DEBUG   2023-07-12T22:17:03-05:00  -  found in Chart.yaml, but missing in charts/ directory: redis-ha
  DEBUG   2023-07-12T22:17:03-05:00  -  unable to add component: error creating chart archive, unable to pull the chart from git: found in Chart.yaml, but missing in charts/ directory: redis-ha
     ERROR:  Failed to create package: unable to add component: error creating chart archive, unable to pull the
             chart from git: found in Chart.yaml, but missing in charts/ directory: redis-ha
  DEBUG   2023-07-12T22:17:03-05:00  -  goroutine 1 [running]:
          runtime/debug.Stack()
          	/usr/local/opt/go/libexec/src/runtime/debug/stack.go:24 +0x64
          github.com/defenseunicorns/zarf/src/pkg/message.Fatal({0x108e2ca40?, 0x1400095c3c0?}, {0x140009f8480, 0xb3})
          	/Users/dgershman/Projects/go/src/github.com/defenseunicorns/zarf/src/pkg/message/message.go:176 +0x128
          github.com/defenseunicorns/zarf/src/pkg/message.Fatalf({0x108e2ca40, 0x1400095c3c0}, {0x1070da294?, 0x0?}, {0x14001ad3b80?, 0x1400056c140?, 0x14000492c30?})
          	/Users/dgershman/Projects/go/src/github.com/defenseunicorns/zarf/src/pkg/message/message.go:183 +0x7c
          github.com/defenseunicorns/zarf/src/cmd.glob..func15(0x10b7089a0?, {0x140005d1d10?, 0x3?, 0x3?})
          	/Users/dgershman/Projects/go/src/github.com/defenseunicorns/zarf/src/cmd/package.go:68 +0x4ac
          github.com/spf13/cobra.(*Command).execute(0x10b7089a0, {0x140005d1cb0, 0x3, 0x3})
          	/Users/dgershman/Projects/go/pkg/mod/github.com/spf13/[email protected]/command.go:944 +0x5b0
          github.com/spf13/cobra.(*Command).ExecuteC(0x10b709800)
          	/Users/dgershman/Projects/go/pkg/mod/github.com/spf13/[email protected]/command.go:1068 +0x35c
          github.com/spf13/cobra.(*Command).Execute(...)
          	/Users/dgershman/Projects/go/pkg/mod/github.com/spf13/[email protected]/command.go:992
          github.com/defenseunicorns/zarf/src/cmd.Execute()
          	/Users/dgershman/Projects/go/src/github.com/defenseunicorns/zarf/src/cmd/root.go:70 +0x28
          main.main()
          	/Users/dgershman/Projects/go/src/github.com/defenseunicorns/zarf/main.go:23 +0x9c

dgershman · 2023-07-13T03:29:23Z

Here is the yaml snippet from zarf.yaml to reproduce the issue:

  - name: argocd-git-chart
    required: true
    charts:
      - name: argocd
        version: argo-cd-5.37.1
        namespace: argocd
        url: https://github.com/argoproj/argo-helm.git
        gitPath: charts/argo-cd
    images:
      - quay.io/argoproj/argocd:v2.7.7
      - public.ecr.aws/docker/library/redis:7.0.11-alpine

Racer159 · 2023-07-20T17:19:09Z

Ah I see - it is actually trying to work but is returning the wrong error - I will push up a fix in a PR shortly. This worked for the git chart for me:

kind: ZarfPackageConfig
metadata:
  name: argocd

components:
  - name: argocd-git-chart
    required: true
    charts:
      - name: argocd
        version: 5.37.1
        namespace: argocd
        url: https://github.com/argoproj/[email protected]
        gitPath: charts/argo-cd
    images:
      - quay.io/argoproj/argocd:v2.7.7
      - public.ecr.aws/docker/library/redis:7.0.11-alpine

(note the version and the @ at the end of the git repo)

Note: I did have to run helm repo add dandydeveloper https://dandydeveloper.github.io/charts/ before building so that Helm inside Zarf could properly build the dependencies. We had an error message that was trying to be helpful but this was not being returned correctly here.

dgershman · 2023-07-20T17:21:23Z

It seems strange to me that we’d have to add the helm repo if we are simply using git. We worked around it, by directly referencing the helm repo, so I’m not blocked by this. I am at the point where I can automate this for the tests, hadn’t gotten around to it quite yet. I hope to pick it up soon. From: Wayne Starr ***@***.***> Date: Thursday, July 20, 2023 at 1:19 PM To: defenseunicorns/zarf ***@***.***> Cc: Danny Gershman ***@***.***>, Author ***@***.***> Subject: Re: [defenseunicorns/zarf] Draft: Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD (PR #1875) Ah I see - it is actually trying to work but is returning the wrong error - I will push up a fix in a PR shortly. This worked for the git chart for me: kind: ZarfPackageConfig metadata: name: argocd components: - name: argocd-git-chart required: true charts: - name: argocd version: 5.37.1 namespace: argocd url: ***@***.*** gitPath: charts/argo-cd images: - quay.io/argoproj/argocd:v2.7.7 - public.ecr.aws/docker/library/redis:7.0.11-alpine (note the version and the @ at the end of the git repo) Note: I did have to run helm repo add dandydeveloper https://dandydeveloper.github.io/charts/ before building so that Helm inside Zarf could properly build the dependencies. We had an error message that was trying to be helpful but this was not being returned correctly here. — Reply to this email directly, view it on GitHub<#1875 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAOD3OYWXWRAIUI7XHB4XXLXRFSBPANCNFSM6AAAAAAZ3RGRGE>. You are receiving this because you authored the thread.Message ID: ***@***.***>

Racer159 · 2023-07-20T17:31:56Z

Yeah we could add it for people when we see them but wanted to behave more like Helm would - at least initially (since those deps are defined as repo deps) - we added a message (which is fixed in #1911) to try to help with that for now

dgershman · 2023-07-23T00:31:48Z

Overall the code looks good! - just a few small suggested changes.

Also you mentioned you had some tests setup, can you add a test under src/test/e2e/22_git_and_flux_test.go for ArgoCD? (potentially renaming that test file to 22_git_and_gitops_test.go)

Also as a note, we have been standardizing around podinfo for most tests to keep things fast / lean (since we only have to pull / push a single image like podinfo:6.4.0 once) but if that is too far out from what you have we can pull in other simple examples for now as well (like guestbook). If you want you could put the package as an example under examples (if you want to add a README.md to it for Argo) or under src/test/packages with a prefix of 22 to just tie it to that test.

Went ahead and built an example for this. I created a new sample app (as the guestbook example seemed a bit fishy). The new sample application is called tracerbullet—essentially it's nginx with a custom message. Let me know if you'd like to have written permissions for this.

I have done as you suggested and renamed the test to be more generic to gitops.

src/test/e2e/22_git_and_gitops_test.go

Racer159 · 2023-08-11T23:01:41Z

Looks like everything worked except for what I feared about storage space - worse comes to worse we can split the build and validate stages across all of our tests (like what Upgrade does) but one thing to try first would be removing images using the new zarf tools registry prune command. I put an example on main for what that would look like with the podinfo flux test (the current merge conflict).

If that doesn't work I'll make another PR to split all of the workflows (will just mean that it will take a few more minutes for us).

Co-authored-by: Wayne Starr <[email protected]>

dgershman · 2023-08-12T02:58:02Z

Looks like everything worked except for what I feared about storage space - worse comes to worse we can split the build and validate stages across all of our tests (like what Upgrade does) but one thing to try first would be removing images using the new zarf tools registry prune command. I put an example on main for what that would look like with the podinfo flux test (the current merge conflict).

If that doesn't work I'll make another PR to split all of the workflows (will just mean that it will take a few more minutes for us).

Nice command. Done.

dgershman · 2023-08-12T16:37:56Z

I have also disabled dex (not required) and there was a left over image (nginx), that isn't needed anymore (for tracerbullet).

Racer159

lgtm

cmwylie19

lgtm

dgershman requested review from jeff-mccoy, YrrepNoj, Racer159 and Noxsios as code owners July 2, 2023 16:32

dgershman changed the title ~~Adds support to mutate repoURL in Application CRD for ArgoCD~~ Draft: Adds support to mutate repoURL in Application CRD for ArgoCD Jul 2, 2023

dgershman changed the title ~~Draft: Adds support to mutate repoURL in Application CRD for ArgoCD~~ Draft: Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD Jul 2, 2023

dgershman changed the title ~~Draft: Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD~~ Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD Jul 2, 2023

Racer159 requested changes Jul 3, 2023

View reviewed changes

dgershman force-pushed the argocd-agent-mutate branch from faa1836 to ede00f7 Compare July 9, 2023 20:02

dgershman force-pushed the argocd-agent-mutate branch from d9552a3 to aa5f361 Compare July 13, 2023 03:04

dgershman force-pushed the argocd-agent-mutate branch from 0314b83 to c4a02c8 Compare July 14, 2023 05:40

dgershman changed the title ~~Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD~~ Draft: Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD Jul 14, 2023

Racer159 mentioned this pull request Jul 20, 2023

Fix the error return for building helm repositories #1911

Merged

5 tasks

dgershman force-pushed the argocd-agent-mutate branch from 0d82eef to 3d09bdc Compare July 22, 2023 21:46

dgershman changed the title ~~Draft: Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD~~ Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD Jul 23, 2023

dgershman requested a review from Racer159 July 25, 2023 18:50

Racer159 requested changes Aug 10, 2023

View reviewed changes

src/test/e2e/22_git_and_gitops_test.go Outdated Show resolved Hide resolved

dgershman and others added 17 commits August 11, 2023 22:46

Adds support to mutate repoURL in Application CRD for ArgoCD

b407729

Adding support for ArgoCD Repositories via Secret

35327f0

removing accidental commit of example

2277d4f

Update src/internal/agent/hooks/argocd-application.go

eacee92

Co-authored-by: Wayne Starr <[email protected]>

changed func

b07f005

include Sources

449d252

adding example for argocd

8da995c

rename function

9f4572f

adding tests for argocd

53de18f

remove arm64 flag

c0df6e5

remove tracerbullet example

0b16b03

making required and moving to podinfo namespace

35cee6e

remove old comment

62d7c8b

decoupling tests, and checking CRDs

8259842

addressing linting issues

3c3d9dc

one more

f5a6b13

fixing missed one

e0b3dfc

dgershman force-pushed the argocd-agent-mutate branch from 784ac37 to e0b3dfc Compare August 12, 2023 02:46

adding prune

9744ade

dgershman requested a review from Madeline-UX as a code owner August 12, 2023 02:57

dgershman requested a review from Racer159 August 12, 2023 02:57

removing unneeded images, disabling things for conserving space

01f64c4

Merge branch 'main' into argocd-agent-mutate

11a0d7d

Racer159 approved these changes Aug 14, 2023

View reviewed changes

cmwylie19 approved these changes Aug 14, 2023

View reviewed changes

Racer159 merged commit 5b5396c into zarf-dev:main Aug 14, 2023
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD #1875

Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD #1875

dgershman commented Jul 2, 2023 •

edited

Loading

netlify bot commented Jul 2, 2023 •

edited

Loading

dgershman commented Jul 2, 2023

dgershman commented Jul 2, 2023

dgershman commented Jul 2, 2023

Racer159 left a comment

dgershman commented Jul 10, 2023

dgershman commented Jul 10, 2023

Racer159 commented Jul 10, 2023

Racer159 commented Jul 12, 2023

dgershman commented Jul 13, 2023

dgershman commented Jul 13, 2023 •

edited

Loading

Racer159 commented Jul 20, 2023 •

edited

Loading

dgershman commented Jul 20, 2023 via email

Racer159 commented Jul 20, 2023

dgershman commented Jul 23, 2023

Racer159 commented Aug 11, 2023

dgershman commented Aug 12, 2023

dgershman commented Aug 12, 2023

Racer159 left a comment

cmwylie19 left a comment

Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD #1875

Adds support to mutate repoURL in Application CRD and Repositories for ArgoCD #1875

Conversation

dgershman commented Jul 2, 2023 • edited Loading

Description

Related Issue

Type of change

Checklist before merging

netlify bot commented Jul 2, 2023 • edited Loading

✅ Deploy Preview for zarf-docs canceled.

dgershman commented Jul 2, 2023

dgershman commented Jul 2, 2023

dgershman commented Jul 2, 2023

Racer159 left a comment

Choose a reason for hiding this comment

dgershman commented Jul 10, 2023

dgershman commented Jul 10, 2023

Racer159 commented Jul 10, 2023

Racer159 commented Jul 12, 2023

dgershman commented Jul 13, 2023

dgershman commented Jul 13, 2023 • edited Loading

Racer159 commented Jul 20, 2023 • edited Loading

dgershman commented Jul 20, 2023 via email

Racer159 commented Jul 20, 2023

dgershman commented Jul 23, 2023

Racer159 commented Aug 11, 2023

dgershman commented Aug 12, 2023

dgershman commented Aug 12, 2023

Racer159 left a comment

Choose a reason for hiding this comment

cmwylie19 left a comment

Choose a reason for hiding this comment

dgershman commented Jul 2, 2023 •

edited

Loading

netlify bot commented Jul 2, 2023 •

edited

Loading

dgershman commented Jul 13, 2023 •

edited

Loading

Racer159 commented Jul 20, 2023 •

edited

Loading