Skip to content

Commit 7b50a34

Browse files
Aamir017Aamir017
authored
Merge branch 'master' into fix/multi-value-flags
2 parents 4cf2437 + 87faf58 commit 7b50a34

File tree

49 files changed

+1218
-348
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1218
-348
lines changed

‎.github/configs/renovate-config.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
module.exports = {
2+
platform: 'github',
3+
gitAuthor: 'renovate[bot] <renovate[bot]@users.noreply.github.com>',
4+
autodiscover: false,
5+
allowPostUpgradeCommandTemplating: true,
6+
allowedPostUpgradeCommands: ["make mockgen"],
7+
extends: [
8+
"github>argoproj/argo-cd//renovate-presets/commons.json5",
9+
"github>argoproj/argo-cd//renovate-presets/custom-managers/shell.json5",
10+
"github>argoproj/argo-cd//renovate-presets/custom-managers/yaml.json5",
11+
"github>argoproj/argo-cd//renovate-presets/fix/disable-all-updates.json5",
12+
"github>argoproj/argo-cd//renovate-presets/devtool.json5",
13+
"github>argoproj/argo-cd//renovate-presets/docs.json5"
14+
]
15+
}

‎.github/workflows/renovate.yaml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
name: Renovate
2+
on:
3+
schedule:
4+
- cron: '0 * * * *'
5+
workflow_dispatch: {}
6+
7+
permissions:
8+
contents: read
9+
10+
jobs:
11+
renovate:
12+
runs-on: ubuntu-latest
13+
steps:
14+
- name: Get token
15+
id: get_token
16+
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1
17+
with:
18+
app-id: ${{ vars.RENOVATE_APP_ID }}
19+
private-key: ${{ secrets.RENOVATE_APP_PRIVATE_KEY }}
20+
21+
- name: Checkout
22+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2
23+
24+
- name: Self-hosted Renovate
25+
uses: renovatebot/github-action@b11417b9eaac3145fe9a8544cee66503724e32b6 #43.0.8
26+
with:
27+
configurationFile: .github/configs/renovate-config.js
28+
token: '${{ steps.get_token.outputs.token }}'
29+
env:
30+
LOG_LEVEL: 'debug'
31+
RENOVATE_REPOSITORIES: '${{ github.repository }}'

‎USERS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ PR with your organization name if you are using Argo CD.
55

66
Currently, the following organizations are **officially** using Argo CD:
77

8+
1. [100ms](https://www.100ms.ai/)
89
1. [127Labs](https://127labs.com/)
910
1. [3Rein](https://www.3rein.com/)
1011
1. [42 School](https://42.fr/)
@@ -339,6 +340,7 @@ Currently, the following organizations are **officially** using Argo CD:
339340
1. [Snapp](https://snapp.ir/)
340341
1. [Snyk](https://snyk.io/)
341342
1. [Softway Medical](https://www.softwaymedical.fr/)
343+
1. [Sophotech](https://sopho.tech)
342344
1. [South China Morning Post (SCMP)](https://www.scmp.com/)
343345
1. [Speee](https://speee.jp/)
344346
1. [Spendesk](https://spendesk.com/)

‎assets/builtin-policy.csv‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# p, <role/user/group>, <resource>, <action>, <object>, <allow/deny>
88

99
p, role:readonly, applications, get, */*, allow
10+
p, role:readonly, applicationsets, get, */*, allow
1011
p, role:readonly, certificates, get, *, allow
1112
p, role:readonly, clusters, get, *, allow
1213
p, role:readonly, repositories, get, *, allow

‎commitserver/commit/commit.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,21 @@ func (s *Service) handleCommitRequest(logCtx *log.Entry, r *apiclient.CommitHydr
118118
return out, "", fmt.Errorf("failed to checkout target branch: %w", err)
119119
}
120120

121-
logCtx.Debug("Clearing repo contents")
122-
out, err = gitClient.RemoveContents()
123-
if err != nil {
124-
return out, "", fmt.Errorf("failed to clear repo: %w", err)
121+
logCtx.Debug("Clearing and preparing paths")
122+
var pathsToClear []string
123+
for _, p := range r.Paths {
124+
if p.Path == "" || p.Path == "." {
125+
logCtx.Debug("Using root directory for manifests, no directory removal needed")
126+
} else {
127+
pathsToClear = append(pathsToClear, p.Path)
128+
}
129+
}
130+
if len(pathsToClear) > 0 {
131+
logCtx.Debugf("Clearing paths: %v", pathsToClear)
132+
out, err := gitClient.RemoveContents(pathsToClear)
133+
if err != nil {
134+
return out, "", fmt.Errorf("failed to clear paths %v: %w", pathsToClear, err)
135+
}
125136
}
126137

127138
logCtx.Debug("Writing manifests")

‎commitserver/commit/commit_test.go‎

Lines changed: 172 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ func Test_CommitHydratedManifests(t *testing.T) {
9999
mockGitClient.On("SetAuthor", "Argo CD", "argo-cd@example.com").Return("", nil).Once()
100100
mockGitClient.On("CheckoutOrOrphan", "env/test", false).Return("", nil).Once()
101101
mockGitClient.On("CheckoutOrNew", "main", "env/test", false).Return("", nil).Once()
102-
mockGitClient.On("RemoveContents").Return("", nil).Once()
103102
mockGitClient.On("CommitAndPush", "main", "test commit message").Return("", nil).Once()
104103
mockGitClient.On("CommitSHA").Return("it-worked!", nil).Once()
105104
mockRepoClientFactory.On("NewClient", mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
@@ -109,6 +108,178 @@ func Test_CommitHydratedManifests(t *testing.T) {
109108
require.NotNil(t, resp)
110109
assert.Equal(t, "it-worked!", resp.HydratedSha)
111110
})
111+
112+
t.Run("root path with dot and blank - no directory removal", func(t *testing.T) {
113+
t.Parallel()
114+
115+
service, mockRepoClientFactory := newServiceWithMocks(t)
116+
mockGitClient := gitmocks.NewClient(t)
117+
mockGitClient.On("Init").Return(nil).Once()
118+
mockGitClient.On("Fetch", mock.Anything).Return(nil).Once()
119+
mockGitClient.On("SetAuthor", "Argo CD", "argo-cd@example.com").Return("", nil).Once()
120+
mockGitClient.On("CheckoutOrOrphan", "env/test", false).Return("", nil).Once()
121+
mockGitClient.On("CheckoutOrNew", "main", "env/test", false).Return("", nil).Once()
122+
mockGitClient.On("CommitAndPush", "main", "test commit message").Return("", nil).Once()
123+
mockGitClient.On("CommitSHA").Return("root-and-blank-sha", nil).Once()
124+
mockRepoClientFactory.On("NewClient", mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
125+
126+
requestWithRootAndBlank := &apiclient.CommitHydratedManifestsRequest{
127+
Repo: &v1alpha1.Repository{
128+
Repo: "https://github.com/argoproj/argocd-example-apps.git",
129+
},
130+
TargetBranch: "main",
131+
SyncBranch: "env/test",
132+
CommitMessage: "test commit message",
133+
Paths: []*apiclient.PathDetails{
134+
{
135+
Path: ".",
136+
Manifests: []*apiclient.HydratedManifestDetails{
137+
{
138+
ManifestJSON: `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"test-dot"}}`,
139+
},
140+
},
141+
},
142+
{
143+
Path: "",
144+
Manifests: []*apiclient.HydratedManifestDetails{
145+
{
146+
ManifestJSON: `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"test-blank"}}`,
147+
},
148+
},
149+
},
150+
},
151+
}
152+
153+
resp, err := service.CommitHydratedManifests(t.Context(), requestWithRootAndBlank)
154+
require.NoError(t, err)
155+
require.NotNil(t, resp)
156+
assert.Equal(t, "root-and-blank-sha", resp.HydratedSha)
157+
})
158+
159+
t.Run("subdirectory path - triggers directory removal", func(t *testing.T) {
160+
t.Parallel()
161+
162+
service, mockRepoClientFactory := newServiceWithMocks(t)
163+
mockGitClient := gitmocks.NewClient(t)
164+
mockGitClient.On("Init").Return(nil).Once()
165+
mockGitClient.On("Fetch", mock.Anything).Return(nil).Once()
166+
mockGitClient.On("SetAuthor", "Argo CD", "argo-cd@example.com").Return("", nil).Once()
167+
mockGitClient.On("CheckoutOrOrphan", "env/test", false).Return("", nil).Once()
168+
mockGitClient.On("CheckoutOrNew", "main", "env/test", false).Return("", nil).Once()
169+
mockGitClient.On("RemoveContents", []string{"apps/staging"}).Return("", nil).Once()
170+
mockGitClient.On("CommitAndPush", "main", "test commit message").Return("", nil).Once()
171+
mockGitClient.On("CommitSHA").Return("subdir-path-sha", nil).Once()
172+
mockRepoClientFactory.On("NewClient", mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
173+
174+
requestWithSubdirPath := &apiclient.CommitHydratedManifestsRequest{
175+
Repo: &v1alpha1.Repository{
176+
Repo: "https://github.com/argoproj/argocd-example-apps.git",
177+
},
178+
TargetBranch: "main",
179+
SyncBranch: "env/test",
180+
CommitMessage: "test commit message",
181+
Paths: []*apiclient.PathDetails{
182+
{
183+
Path: "apps/staging", // subdirectory path
184+
Manifests: []*apiclient.HydratedManifestDetails{
185+
{
186+
ManifestJSON: `{"apiVersion":"v1","kind":"Deployment","metadata":{"name":"test-app"}}`,
187+
},
188+
},
189+
},
190+
},
191+
}
192+
193+
resp, err := service.CommitHydratedManifests(t.Context(), requestWithSubdirPath)
194+
require.NoError(t, err)
195+
require.NotNil(t, resp)
196+
assert.Equal(t, "subdir-path-sha", resp.HydratedSha)
197+
})
198+
199+
t.Run("mixed paths - root and subdirectory", func(t *testing.T) {
200+
t.Parallel()
201+
202+
service, mockRepoClientFactory := newServiceWithMocks(t)
203+
mockGitClient := gitmocks.NewClient(t)
204+
mockGitClient.On("Init").Return(nil).Once()
205+
mockGitClient.On("Fetch", mock.Anything).Return(nil).Once()
206+
mockGitClient.On("SetAuthor", "Argo CD", "argo-cd@example.com").Return("", nil).Once()
207+
mockGitClient.On("CheckoutOrOrphan", "env/test", false).Return("", nil).Once()
208+
mockGitClient.On("CheckoutOrNew", "main", "env/test", false).Return("", nil).Once()
209+
mockGitClient.On("RemoveContents", []string{"apps/production", "apps/staging"}).Return("", nil).Once()
210+
mockGitClient.On("CommitAndPush", "main", "test commit message").Return("", nil).Once()
211+
mockGitClient.On("CommitSHA").Return("mixed-paths-sha", nil).Once()
212+
mockRepoClientFactory.On("NewClient", mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
213+
214+
requestWithMixedPaths := &apiclient.CommitHydratedManifestsRequest{
215+
Repo: &v1alpha1.Repository{
216+
Repo: "https://github.com/argoproj/argocd-example-apps.git",
217+
},
218+
TargetBranch: "main",
219+
SyncBranch: "env/test",
220+
CommitMessage: "test commit message",
221+
Paths: []*apiclient.PathDetails{
222+
{
223+
Path: ".", // root path - should NOT trigger removal
224+
Manifests: []*apiclient.HydratedManifestDetails{
225+
{
226+
ManifestJSON: `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"global-config"}}`,
227+
},
228+
},
229+
},
230+
{
231+
Path: "apps/production", // subdirectory path - SHOULD trigger removal
232+
Manifests: []*apiclient.HydratedManifestDetails{
233+
{
234+
ManifestJSON: `{"apiVersion":"v1","kind":"Deployment","metadata":{"name":"prod-app"}}`,
235+
},
236+
},
237+
},
238+
{
239+
Path: "apps/staging", // another subdirectory path - SHOULD trigger removal
240+
Manifests: []*apiclient.HydratedManifestDetails{
241+
{
242+
ManifestJSON: `{"apiVersion":"v1","kind":"Deployment","metadata":{"name":"staging-app"}}`,
243+
},
244+
},
245+
},
246+
},
247+
}
248+
249+
resp, err := service.CommitHydratedManifests(t.Context(), requestWithMixedPaths)
250+
require.NoError(t, err)
251+
require.NotNil(t, resp)
252+
assert.Equal(t, "mixed-paths-sha", resp.HydratedSha)
253+
})
254+
255+
t.Run("empty paths array", func(t *testing.T) {
256+
t.Parallel()
257+
258+
service, mockRepoClientFactory := newServiceWithMocks(t)
259+
mockGitClient := gitmocks.NewClient(t)
260+
mockGitClient.On("Init").Return(nil).Once()
261+
mockGitClient.On("Fetch", mock.Anything).Return(nil).Once()
262+
mockGitClient.On("SetAuthor", "Argo CD", "argo-cd@example.com").Return("", nil).Once()
263+
mockGitClient.On("CheckoutOrOrphan", "env/test", false).Return("", nil).Once()
264+
mockGitClient.On("CheckoutOrNew", "main", "env/test", false).Return("", nil).Once()
265+
mockGitClient.On("CommitAndPush", "main", "test commit message").Return("", nil).Once()
266+
mockGitClient.On("CommitSHA").Return("it-worked!", nil).Once()
267+
mockRepoClientFactory.On("NewClient", mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
268+
269+
requestWithEmptyPaths := &apiclient.CommitHydratedManifestsRequest{
270+
Repo: &v1alpha1.Repository{
271+
Repo: "https://github.com/argoproj/argocd-example-apps.git",
272+
},
273+
TargetBranch: "main",
274+
SyncBranch: "env/test",
275+
CommitMessage: "test commit message",
276+
}
277+
278+
resp, err := service.CommitHydratedManifests(t.Context(), requestWithEmptyPaths)
279+
require.NoError(t, err)
280+
require.NotNil(t, resp)
281+
assert.Equal(t, "it-worked!", resp.HydratedSha)
282+
})
112283
}
113284

114285
func newServiceWithMocks(t *testing.T) (*Service, *mocks.RepoClientFactory) {

‎commitserver/commit/hydratorhelper.go‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ package commit
22

33
import (
44
"encoding/json"
5-
"errors"
65
"fmt"
7-
"io/fs"
86
"os"
97
"path/filepath"
108
"strings"
@@ -73,7 +71,7 @@ func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *app
7371
hydratePath = ""
7472
}
7573

76-
err = mkdirAll(root, hydratePath)
74+
err = root.MkdirAll(hydratePath, 0o755)
7775
if err != nil {
7876
return fmt.Errorf("failed to create path: %w", err)
7977
}
@@ -212,25 +210,3 @@ func writeManifests(root *os.Root, dirPath string, manifests []*apiclient.Hydrat
212210

213211
return nil
214212
}
215-
216-
// mkdirAll creates the directory and all its parents if they do not exist. It returns an error if the directory
217-
// cannot be.
218-
func mkdirAll(root *os.Root, dirPath string) error {
219-
parts := strings.Split(dirPath, string(os.PathSeparator))
220-
builtPath := ""
221-
for _, part := range parts {
222-
if part == "" {
223-
continue
224-
}
225-
builtPath = filepath.Join(builtPath, part)
226-
err := root.Mkdir(builtPath, os.ModePerm)
227-
if err != nil {
228-
if errors.Is(err, fs.ErrExist) {
229-
log.WithError(err).Warnf("path %s already exists, skipping", dirPath)
230-
continue
231-
}
232-
return fmt.Errorf("failed to create path: %w", err)
233-
}
234-
}
235-
return nil
236-
}

‎common/common.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ const (
192192
LabelValueSecretTypeRepoCreds = "repo-creds"
193193
// LabelValueSecretTypeRepositoryWrite indicates a secret type of repository credentials for writing
194194
LabelValueSecretTypeRepositoryWrite = "repository-write"
195+
// LabelValueSecretTypeRepoCredsWrite indicates a secret type of repository credentials for writing for templating
196+
LabelValueSecretTypeRepoCredsWrite = "repo-write-creds"
195197
// LabelValueSecretTypeSCMCreds indicates a secret type of SCM credentials
196198
LabelValueSecretTypeSCMCreds = "scm-creds"
197199

‎controller/hydrator/hydrator.go‎

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -136,21 +136,6 @@ func getHydrationQueueKey(app *appv1.Application) types.HydrationQueueKey {
136136
return key
137137
}
138138

139-
// uniqueHydrationDestination is used to detect duplicate hydrate destinations.
140-
type uniqueHydrationDestination struct {
141-
// sourceRepoURL must be normalized with git.NormalizeGitURL to ensure that two apps with different URL formats
142-
// don't end up in two different hydration queue items. Failing to normalize would result in one hydrated commit for
143-
// each unique URL.
144-
//nolint:unused // used as part of a map key
145-
sourceRepoURL string
146-
//nolint:unused // used as part of a map key
147-
sourceTargetRevision string
148-
//nolint:unused // used as part of a map key
149-
destinationBranch string
150-
//nolint:unused // used as part of a map key
151-
destinationPath string
152-
}
153-
154139
// ProcessHydrationQueueItem processes a hydration queue item. It retrieves the relevant applications for the given
155140
// hydration key, hydrates their latest commit, and updates their status accordingly. If the hydration fails, it marks
156141
// the operation as failed and logs the error. If successful, it updates the operation to indicate that hydration was
@@ -234,7 +219,7 @@ func (h *Hydrator) getRelevantAppsForHydration(logCtx *log.Entry, hydrationKey t
234219
}
235220

236221
var relevantApps []*appv1.Application
237-
uniqueDestinations := make(map[uniqueHydrationDestination]bool, len(apps.Items))
222+
uniquePaths := make(map[string]bool, len(apps.Items))
238223
for _, app := range apps.Items {
239224
if app.Spec.SourceHydrator == nil {
240225
continue
@@ -264,17 +249,12 @@ func (h *Hydrator) getRelevantAppsForHydration(logCtx *log.Entry, hydrationKey t
264249
continue
265250
}
266251

267-
uniqueDestinationKey := uniqueHydrationDestination{
268-
sourceRepoURL: git.NormalizeGitURLAllowInvalid(app.Spec.SourceHydrator.DrySource.RepoURL),
269-
sourceTargetRevision: app.Spec.SourceHydrator.DrySource.TargetRevision,
270-
destinationBranch: destinationBranch,
271-
destinationPath: app.Spec.SourceHydrator.SyncSource.Path,
272-
}
273252
// TODO: test the dupe detection
274-
if _, ok := uniqueDestinations[uniqueDestinationKey]; ok {
275-
return nil, fmt.Errorf("multiple app hydrators use the same destination: %v", uniqueDestinationKey)
253+
// TODO: normalize the path to avoid "path/.." from being treated as different from "."
254+
if _, ok := uniquePaths[app.Spec.SourceHydrator.SyncSource.Path]; ok {
255+
return nil, fmt.Errorf("multiple app hydrators use the same destination: %v", app.Spec.SourceHydrator.SyncSource.Path)
276256
}
277-
uniqueDestinations[uniqueDestinationKey] = true
257+
uniquePaths[app.Spec.SourceHydrator.SyncSource.Path] = true
278258

279259
relevantApps = append(relevantApps, &app)
280260
}

0 commit comments

Comments
 (0)