This commit is contained in:
Fredrik Larsson 2026-04-21 06:09:12 +03:00 committed by GitHub
commit b4561fb4a2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 335 additions and 0 deletions

View file

@ -1813,6 +1813,110 @@ func shortenRevision(revision string, length int) string {
return revision
}
// buildEnvForRevision builds an Env containing only the revision-derived variables, which are the
// only ones that can differ between two revisions of the same application source. This is used by
// buildEnvHasChanged to compare parameter substitution results across revisions without needing a
// full ManifestRequest.
func buildEnvForRevision(appSource *v1alpha1.ApplicationSource, repo *v1alpha1.Repository, revision string) *v1alpha1.Env {
shortRevision := shortenRevision(revision, 7)
shortRevision8 := shortenRevision(revision, 8)
repoURL := ""
if repo != nil {
repoURL = repo.Repo
}
sourcePath := ""
sourceTargetRevision := ""
if appSource != nil {
sourcePath = appSource.Path
sourceTargetRevision = appSource.TargetRevision
}
return &v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION", Value: revision},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION_SHORT", Value: shortRevision},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION_SHORT_8", Value: shortRevision8},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_SOURCE_REPO_URL", Value: repoURL},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_SOURCE_PATH", Value: sourcePath},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_SOURCE_TARGET_REVISION", Value: sourceTargetRevision},
}
}
// buildEnvHasChanged returns true if any application source parameter that undergoes environment
// variable substitution during manifest generation would produce a different value between
// oldRevision and newRevision. When true, manifest regeneration is required even if no files in
// the manifest-generate-paths have changed.
func buildEnvHasChanged(appSource *v1alpha1.ApplicationSource, repo *v1alpha1.Repository, oldRevision, newRevision string) bool {
if appSource == nil {
return false
}
oldEnv := buildEnvForRevision(appSource, repo, oldRevision)
newEnv := buildEnvForRevision(appSource, repo, newRevision)
return appParametersWouldChange(appSource, oldEnv, newEnv)
}
// appParametersWouldChange checks whether environment substitution would produce different results
// for any application source parameter between two environment contexts.
func appParametersWouldChange(appSource *v1alpha1.ApplicationSource, oldEnv, newEnv *v1alpha1.Env) bool {
envChanged := func(value string) bool {
return oldEnv.Envsubst(value) != newEnv.Envsubst(value)
}
if appSource.Helm != nil {
for _, param := range appSource.Helm.Parameters {
if envChanged(param.Value) {
return true
}
}
for _, fileParam := range appSource.Helm.FileParameters {
if envChanged(fileParam.Path) {
return true
}
}
}
if appSource.Kustomize != nil {
for _, image := range appSource.Kustomize.Images {
if envChanged(string(image)) {
return true
}
}
for _, v := range appSource.Kustomize.CommonLabels {
if envChanged(v) {
return true
}
}
if appSource.Kustomize.CommonAnnotationsEnvsubst {
for _, v := range appSource.Kustomize.CommonAnnotations {
if envChanged(v) {
return true
}
}
}
}
if appSource.Directory != nil && !appSource.Directory.Jsonnet.IsZero() {
for _, tla := range appSource.Directory.Jsonnet.TLAs {
if envChanged(tla.Value) {
return true
}
}
for _, extVar := range appSource.Directory.Jsonnet.ExtVars {
if envChanged(extVar.Value) {
return true
}
}
}
if appSource.Plugin != nil {
for _, envEntry := range appSource.Plugin.Env {
if envChanged(envEntry.Value) {
return true
}
}
}
return false
}
func newEnvRepoQuery(q *apiclient.RepoServerAppDetailsQuery, revision string) *v1alpha1.Env {
return &v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_NAME", Value: q.AppName},
@ -3456,6 +3560,17 @@ func (s *Service) UpdateRevisionForPaths(_ context.Context, request *apiclient.U
}, nil
}
// Even when no files in manifest-generate-paths changed, build environment variables like
// $ARGOCD_APP_REVISION that are used in parameters will produce different values for the new
// revision, so we must regenerate.
if buildEnvHasChanged(request.ApplicationSource, request.GetRepo(), sRevision, rRevision) {
logCtx.Debugf("build environment variable changes detected for application %s from revision %s to %s", request.AppName, sRevision, rRevision)
return &apiclient.UpdateRevisionForPathsResponse{
Revision: rRevision,
Changes: true,
}, nil
}
// No changes detected, update the cache using resolved revisions
err := s.updateCachedRevision(logCtx, sRevision, rRevision, request, oldRepoRefs, newRepoRefs)
if err != nil {

View file

@ -5127,6 +5127,96 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, want: &apiclient.UpdateRevisionForPathsResponse{
Revision: "0.0.1", Changes: true,
}, wantErr: assert.Error, cacheHit: nil},
{name: "HelmParamWithEnvVarChangesRevision", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().LsRemote("HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().LsRemote("SYNCEDHEAD").Once().Return("1e67a504d03def3a6a1125d934cb511680f72555", nil)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
paths.EXPECT().GetPathIfExists(mock.Anything).Return(".")
gitClient.EXPECT().Root().Return("")
gitClient.EXPECT().ChangedFiles(mock.Anything, mock.Anything).Return([]string{}, nil)
}, ".")
return fields{service: s, cache: c}
}(), args: args{
ctx: t.Context(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &v1alpha1.Repository{Repo: "a-url.com", Type: "git"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
ApplicationSource: &v1alpha1.ApplicationSource{
Path: ".",
Helm: &v1alpha1.ApplicationSourceHelm{
Parameters: []v1alpha1.HelmParameter{
{Name: "image.tag", Value: "$ARGOCD_APP_REVISION"},
},
},
},
},
}, want: &apiclient.UpdateRevisionForPathsResponse{
Revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
Changes: true,
}, wantErr: assert.NoError, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 0,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "HelmParamWithNoEnvVarNoExtraChanges", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Init().Return(nil)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("632039659e542ed7de0c170a4fcc1c571b288fc0").Once().Return(false)
gitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(false)
gitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Once().Return(nil)
gitClient.EXPECT().IsRevisionPresent("1e67a504d03def3a6a1125d934cb511680f72555").Once().Return(true)
gitClient.EXPECT().LsRemote("HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.EXPECT().LsRemote("SYNCEDHEAD").Once().Return("1e67a504d03def3a6a1125d934cb511680f72555", nil)
paths.EXPECT().GetPath(mock.Anything).Return(".", nil)
paths.EXPECT().GetPathIfExists(mock.Anything).Return(".")
gitClient.EXPECT().Root().Return("")
gitClient.EXPECT().ChangedFiles(mock.Anything, mock.Anything).Return([]string{}, nil)
}, ".")
return fields{service: s, cache: c}
}(), args: args{
ctx: t.Context(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &v1alpha1.Repository{Repo: "a-url.com", Type: "git"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
AppLabelKey: "app.kubernetes.io/name",
AppName: "static-param-app",
Namespace: "default",
TrackingMethod: "annotation+label",
ApplicationSource: &v1alpha1.ApplicationSource{
Path: ".",
Helm: &v1alpha1.ApplicationSourceHelm{
Parameters: []v1alpha1.HelmParameter{
{Name: "replicas", Value: "3"},
},
},
},
KubeVersion: "v1.16.0",
},
}, want: &apiclient.UpdateRevisionForPathsResponse{
Revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
Changes: true, // FIXME: need to fix changes=true, because now test can't mock Rename cache
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "1e67a504d03def3a6a1125d934cb511680f72555",
revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "IgnoreRefSourcesForGitSource", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Init().Return(nil)
@ -5208,6 +5298,136 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}
}
func TestAppParametersWouldChange(t *testing.T) {
oldEnv := &v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION", Value: "aaaaaaa"},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION_SHORT", Value: "aaaaaaa"},
}
newEnv := &v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION", Value: "bbbbbbb"},
&v1alpha1.EnvEntry{Name: "ARGOCD_APP_REVISION_SHORT", Value: "bbbbbbb"},
}
t.Run("HelmParamWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Helm: &v1alpha1.ApplicationSourceHelm{
Parameters: []v1alpha1.HelmParameter{
{Name: "image.tag", Value: "$ARGOCD_APP_REVISION"},
},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("HelmParamWithStaticValue", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Helm: &v1alpha1.ApplicationSourceHelm{
Parameters: []v1alpha1.HelmParameter{
{Name: "replicas", Value: "3"},
},
},
}
assert.False(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("HelmFileParamWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Helm: &v1alpha1.ApplicationSourceHelm{
FileParameters: []v1alpha1.HelmFileParameter{
{Name: "config", Path: "configs/$ARGOCD_APP_REVISION/values.yaml"},
},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("KustomizeImageWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Kustomize: &v1alpha1.ApplicationSourceKustomize{
Images: v1alpha1.KustomizeImages{"myrepo/myimage:$ARGOCD_APP_REVISION"},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("KustomizeCommonLabelWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Kustomize: &v1alpha1.ApplicationSourceKustomize{
CommonLabels: map[string]string{"revision": "$ARGOCD_APP_REVISION"},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("KustomizeCommonAnnotationWithEnvVarAndEnvsubstEnabled", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Kustomize: &v1alpha1.ApplicationSourceKustomize{
CommonAnnotations: map[string]string{"revision": "$ARGOCD_APP_REVISION"},
CommonAnnotationsEnvsubst: true,
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("KustomizeCommonAnnotationWithEnvVarButEnvsubstDisabled", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Kustomize: &v1alpha1.ApplicationSourceKustomize{
CommonAnnotations: map[string]string{"revision": "$ARGOCD_APP_REVISION"},
CommonAnnotationsEnvsubst: false,
},
}
assert.False(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("JsonnetTLAWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Directory: &v1alpha1.ApplicationSourceDirectory{
Jsonnet: v1alpha1.ApplicationSourceJsonnet{
TLAs: []v1alpha1.JsonnetVar{{Name: "revision", Value: "$ARGOCD_APP_REVISION"}},
},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("JsonnetExtVarWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Directory: &v1alpha1.ApplicationSourceDirectory{
Jsonnet: v1alpha1.ApplicationSourceJsonnet{
ExtVars: []v1alpha1.JsonnetVar{{Name: "revision", Value: "$ARGOCD_APP_REVISION"}},
},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("PluginEnvWithRevisionEnvVar", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Plugin: &v1alpha1.ApplicationSourcePlugin{
Env: v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "MY_REVISION", Value: "$ARGOCD_APP_REVISION"},
},
},
}
assert.True(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("PluginEnvWithStaticValue", func(t *testing.T) {
source := &v1alpha1.ApplicationSource{
Plugin: &v1alpha1.ApplicationSourcePlugin{
Env: v1alpha1.Env{
&v1alpha1.EnvEntry{Name: "ENV", Value: "production"},
},
},
}
assert.False(t, appParametersWouldChange(source, oldEnv, newEnv))
})
t.Run("NilSource", func(t *testing.T) {
assert.False(t, appParametersWouldChange(&v1alpha1.ApplicationSource{}, oldEnv, newEnv))
})
}
func Test_getRepoSanitizerRegex(t *testing.T) {
r := getRepoSanitizerRegex("/tmp/_argocd-repo")
msg := r.ReplaceAllString("error message containing /tmp/_argocd-repo/SENSITIVE and other stuff", "<path to cached source>")