feat(controller): use manifest generate path during comparison (#14242) (#15636)

* squash commits

Signed-off-by: Alexy Mantha <[email protected]>

* Update util/git/client.go

Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Alexy Mantha <[email protected]>

* fix error message

Signed-off-by: Alexy Mantha <[email protected]>

* add git client options

Signed-off-by: Alexy Mantha <[email protected]>

* Update generated code

Signed-off-by: Alexy Mantha <[email protected]>

* run fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* failed gen

Signed-off-by: Alexy Mantha <[email protected]>

* tweak logs and rename cache

Signed-off-by: Alexy Mantha <[email protected]>

* validate revisions

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix linting

Signed-off-by: Alexy Mantha <[email protected]>

* fixes from review

Signed-off-by: Alexy Mantha <[email protected]>

* generate

Signed-off-by: Alexy Mantha <[email protected]>

* fix

Signed-off-by: Alexy Mantha <[email protected]>

* use log context

Signed-off-by: Alexy Mantha <[email protected]>

---------

Signed-off-by: Alexy Mantha <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
This commit is contained in:
Alexy Mantha
2024-04-04 16:01:11 -04:00
committed by GitHub
co-authored by Michael Crenshaw
parent 6f5c13bcb3
commit cf3b2fd8ad
19 changed files with 2148 additions and 301 deletions
+11 -8
View File
@@ -53,14 +53,15 @@ type namespacedResource struct {
}
type fakeData struct {
apps []runtime.Object
manifestResponse *apiclient.ManifestResponse
manifestResponses []*apiclient.ManifestResponse
managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured
namespacedResources map[kube.ResourceKey]namespacedResource
configMapData map[string]string
metricsCacheExpiration time.Duration
applicationNamespaces []string
apps []runtime.Object
manifestResponse *apiclient.ManifestResponse
manifestResponses []*apiclient.ManifestResponse
managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured
namespacedResources map[kube.ResourceKey]namespacedResource
configMapData map[string]string
metricsCacheExpiration time.Duration
applicationNamespaces []string
updateRevisionForPathsResponse *apiclient.UpdateRevisionForPathsResponse
}
type MockKubectl struct {
@@ -106,6 +107,8 @@ func newFakeController(data *fakeData, repoErr error) *ApplicationController {
}
}
mockRepoClient.On("UpdateRevisionForPaths", mock.Anything, mock.Anything).Return(data.updateRevisionForPathsResponse, nil)
mockRepoClientset := mockrepoclient.Clientset{RepoServerServiceClient: &mockRepoClient}
secret := corev1.Secret{
+33
View File
@@ -33,6 +33,7 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
"github.com/argoproj/argo-cd/v2/util/app/path"
"github.com/argoproj/argo-cd/v2/util/argo"
argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff"
appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate"
@@ -194,6 +195,38 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
return nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
}
syncedRevision := app.Status.Sync.Revision
if app.Spec.HasMultipleSources() {
if i < len(app.Status.Sync.Revisions) {
syncedRevision = app.Status.Sync.Revisions[i]
} else {
syncedRevision = ""
}
}
val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]
if !source.IsHelm() && syncedRevision != "" && ok && val != "" {
// Validate the manifest-generate-path annotation to avoid generating manifests if it has not changed.
_, err = repoClient.UpdateRevisionForPaths(context.Background(), &apiclient.UpdateRevisionForPathsRequest{
Repo: repo,
Revision: revisions[i],
SyncedRevision: syncedRevision,
Paths: path.GetAppRefreshPaths(app),
AppLabelKey: appLabelKey,
AppName: app.InstanceName(m.namespace),
Namespace: app.Spec.Destination.Namespace,
ApplicationSource: &source,
KubeVersion: serverVersion,
ApiVersions: argo.APIResourcesToStrings(apiResources, true),
TrackingMethod: string(argo.GetTrackingMethod(m.settingsMgr)),
RefSources: refSources,
HasMultipleSources: app.Spec.HasMultipleSources(),
})
if err != nil {
return nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err)
}
}
ts.AddCheckpoint("version_ms")
log.Debugf("Generating Manifest for source %s revision %s", source, revisions[i])
manifestInfo, err := repoClient.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+32
View File
@@ -27,6 +27,7 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
mockrepoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks"
"github.com/argoproj/argo-cd/v2/test"
"github.com/argoproj/argo-cd/v2/util/argo"
)
@@ -651,6 +652,37 @@ var defaultProj = argoappv1.AppProject{
},
}
// TestCompareAppStateWithManifestGeneratePath tests that it compares revisions when the manifest-generate-path annotation is set.
func TestCompareAppStateWithManifestGeneratePath(t *testing.T) {
app := newFakeApp()
app.SetAnnotations(map[string]string{argoappv1.AnnotationKeyManifestGeneratePaths: "."})
app.Status.Sync = argoappv1.SyncStatus{
Revision: "abc123",
Status: argoappv1.SyncStatusCodeSynced,
}
data := fakeData{
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
updateRevisionForPathsResponse: &apiclient.UpdateRevisionForPathsResponse{},
}
ctrl := newFakeController(&data, nil)
revisions := make([]string, 0)
revisions = append(revisions, "abc123")
compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false)
assert.Nil(t, err)
assert.NotNil(t, compRes)
assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
assert.Equal(t, "abc123", compRes.syncStatus.Revision)
ctrl.repoClientset.(*mockrepoclient.Clientset).RepoServerServiceClient.(*mockrepoclient.RepoServerServiceClient).AssertNumberOfCalls(t, "UpdateRevisionForPaths", 1)
}
func TestSetHealth(t *testing.T) {
app := newFakeApp()
deployment := kube.MustToUnstructured(&v1.Deployment{
+5 -3
View File
@@ -178,17 +178,19 @@ If the manifest generation has no side effects then requests are processed in pa
* **Multiple Kustomize applications in same repository with [parameter overrides](../user-guide/parameters.md):** sorry, no workaround for now.
### Webhook and Manifest Paths Annotation
### Manifest Paths Annotation
Argo CD aggressively caches generated manifests and uses the repository commit SHA as a cache key. A new commit to the Git repository invalidates the cache for all applications configured in the repository.
This can negatively affect repositories with multiple applications. You can use [webhooks](https://github.com/argoproj/argo-cd/blob/master/docs/operator-manual/webhook.md) and the `argocd.argoproj.io/manifest-generate-paths` Application CRD annotation to solve this problem and improve performance.
The `argocd.argoproj.io/manifest-generate-paths` annotation contains a semicolon-separated list of paths within the Git repository that are used during manifest generation. The webhook compares paths specified in the annotation with the changed files specified in the webhook payload. If no modified files match the paths specified in `argocd.argoproj.io/manifest-generate-paths`, then the webhook will not trigger application reconciliation and the existing cache will be considered valid for the new commit.
The `argocd.argoproj.io/manifest-generate-paths` annotation contains a semicolon-separated list of paths within the Git repository that are used during manifest generation. It will use the paths specified in the annotation to compare the last cached revision to the latest commit. If no modified files match the paths specified in `argocd.argoproj.io/manifest-generate-paths`, then it will not trigger application reconciliation and the existing cache will be considered valid for the new commit.
Installations that use a different repository for each application are **not** subject to this behavior and will likely get no benefit from using these annotations.
For webhooks, the comparison is done using the files specified in the webhook event payload instead.
!!! note
Application manifest paths annotation support depends on the git provider used for the Application. It is currently only supported for GitHub, GitLab, and Gogs based repos.
Application manifest paths annotation support for webhooks depends on the git provider used for the Application. It is currently only supported for GitHub, GitLab, and Gogs based repos.
* **Relative path** The annotation might contain a relative path. In this case the path is considered relative to the path specified in the application source:
@@ -1,4 +1,4 @@
// Code generated by mockery v2.21.1. DO NOT EDIT.
// Code generated by mockery v2.32.4. DO NOT EDIT.
package mocks
@@ -231,6 +231,10 @@ func (_m *RepoServerServiceClient) GetRevisionChartDetails(ctx context.Context,
ret := _m.Called(_ca...)
var r0 *v1alpha1.ChartDetails
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, *apiclient.RepoServerRevisionChartDetailsRequest, ...grpc.CallOption) (*v1alpha1.ChartDetails, error)); ok {
return rf(ctx, in, opts...)
}
if rf, ok := ret.Get(0).(func(context.Context, *apiclient.RepoServerRevisionChartDetailsRequest, ...grpc.CallOption) *v1alpha1.ChartDetails); ok {
r0 = rf(ctx, in, opts...)
} else {
@@ -239,7 +243,6 @@ func (_m *RepoServerServiceClient) GetRevisionChartDetails(ctx context.Context,
}
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, *apiclient.RepoServerRevisionChartDetailsRequest, ...grpc.CallOption) error); ok {
r1 = rf(ctx, in, opts...)
} else {
@@ -447,13 +450,45 @@ func (_m *RepoServerServiceClient) TestRepository(ctx context.Context, in *apicl
return r0, r1
}
type mockConstructorTestingTNewRepoServerServiceClient interface {
mock.TestingT
Cleanup(func())
// UpdateRevisionForPaths provides a mock function with given fields: ctx, in, opts
func (_m *RepoServerServiceClient) UpdateRevisionForPaths(ctx context.Context, in *apiclient.UpdateRevisionForPathsRequest, opts ...grpc.CallOption) (*apiclient.UpdateRevisionForPathsResponse, error) {
_va := make([]interface{}, len(opts))
for _i := range opts {
_va[_i] = opts[_i]
}
var _ca []interface{}
_ca = append(_ca, ctx, in)
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
var r0 *apiclient.UpdateRevisionForPathsResponse
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, *apiclient.UpdateRevisionForPathsRequest, ...grpc.CallOption) (*apiclient.UpdateRevisionForPathsResponse, error)); ok {
return rf(ctx, in, opts...)
}
if rf, ok := ret.Get(0).(func(context.Context, *apiclient.UpdateRevisionForPathsRequest, ...grpc.CallOption) *apiclient.UpdateRevisionForPathsResponse); ok {
r0 = rf(ctx, in, opts...)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*apiclient.UpdateRevisionForPathsResponse)
}
}
if rf, ok := ret.Get(1).(func(context.Context, *apiclient.UpdateRevisionForPathsRequest, ...grpc.CallOption) error); ok {
r1 = rf(ctx, in, opts...)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// NewRepoServerServiceClient creates a new instance of RepoServerServiceClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
func NewRepoServerServiceClient(t mockConstructorTestingTNewRepoServerServiceClient) *RepoServerServiceClient {
// The first argument is typically a *testing.T value.
func NewRepoServerServiceClient(t interface {
mock.TestingT
Cleanup(func())
}) *RepoServerServiceClient {
mock := &RepoServerServiceClient{}
mock.Mock.Test(t)
File diff suppressed because it is too large Load Diff
+3
View File
@@ -35,6 +35,7 @@ type CacheCallCounts struct {
ExternalSets int
ExternalGets int
ExternalDeletes int
ExternalRenames int
}
// Checks that the cache was called the expected number of times
@@ -42,12 +43,14 @@ func (mockCache *MockRepoCache) AssertCacheCalledTimes(t *testing.T, calls *Cach
mockCache.RedisClient.AssertNumberOfCalls(t, "Get", calls.ExternalGets)
mockCache.RedisClient.AssertNumberOfCalls(t, "Set", calls.ExternalSets)
mockCache.RedisClient.AssertNumberOfCalls(t, "Delete", calls.ExternalDeletes)
mockCache.RedisClient.AssertNumberOfCalls(t, "Rename", calls.ExternalRenames)
}
func (mockCache *MockRepoCache) ConfigureDefaultCallbacks() {
mockCache.RedisClient.On("Get", mock.Anything, mock.Anything).Return(nil)
mockCache.RedisClient.On("Set", mock.Anything).Return(nil)
mockCache.RedisClient.On("Delete", mock.Anything).Return(nil)
mockCache.RedisClient.On("Rename", mock.Anything, mock.Anything, mock.Anything).Return(nil)
}
func NewInMemoryRedis() (*redis.Client, func()) {
+104
View File
@@ -45,6 +45,7 @@ import (
"github.com/argoproj/argo-cd/v2/reposerver/cache"
"github.com/argoproj/argo-cd/v2/reposerver/metrics"
"github.com/argoproj/argo-cd/v2/util/app/discovery"
apppathutil "github.com/argoproj/argo-cd/v2/util/app/path"
argopath "github.com/argoproj/argo-cd/v2/util/app/path"
"github.com/argoproj/argo-cd/v2/util/argo"
"github.com/argoproj/argo-cd/v2/util/cmp"
@@ -837,6 +838,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
innerRes.NumberOfConsecutiveFailures++
innerRes.MostRecentError = err.Error()
cacheErr = s.cache.SetManifests(cacheKey, appSourceCopy, q.RefSources, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, innerRes, refSourceCommitSHAs)
if cacheErr != nil {
logCtx.Warnf("manifest cache set error %s: %v", appSourceCopy.String(), cacheErr)
ch.errCh <- cacheErr
@@ -2675,3 +2677,105 @@ func (s *Service) GetGitDirectories(_ context.Context, request *apiclient.GitDir
Paths: paths,
}, nil
}
// UpdateRevisionForPaths compares two git revisions and checks if the files in the given paths have changed
// If no files were changed, it will store the already cached manifest to the key corresponding to the old revision, avoiding an unnecessary generation.
// Example: cache has key "a1a1a1" with manifest "x", and the files for that manifest have not changed,
// "x" will be stored again with the new revision "b2b2b2".
func (s *Service) UpdateRevisionForPaths(_ context.Context, request *apiclient.UpdateRevisionForPathsRequest) (*apiclient.UpdateRevisionForPathsResponse, error) {
logCtx := log.WithFields(log.Fields{"application": request.AppName, "appNamespace": request.Namespace})
repo := request.GetRepo()
revision := request.GetRevision()
syncedRevision := request.GetSyncedRevision()
refreshPaths := request.GetPaths()
if repo == nil {
return nil, status.Error(codes.InvalidArgument, "must pass a valid repo")
}
if len(refreshPaths) == 0 {
// Always refresh if path is not specified
return &apiclient.UpdateRevisionForPathsResponse{}, nil
}
gitClientOpts := git.WithCache(s.cache, true)
gitClient, revision, err := s.newClientResolveRevision(repo, revision, gitClientOpts)
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}
syncedRevision, err = gitClient.LsRemote(syncedRevision)
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}
// No need to compare if it is the same revision
if revision == syncedRevision {
return &apiclient.UpdateRevisionForPathsResponse{}, nil
}
s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, false)
})
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s: %v", repo.Repo, revision, err)
}
defer io.Close(closer)
files, err := gitClient.ChangedFiles(syncedRevision, revision)
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to get changed files for repo %s with revision %s: %v", repo.Repo, revision, err)
}
changed := apppathutil.AppFilesHaveChanged(refreshPaths, files)
if !changed {
logCtx.Debugf("no changes found for application %s in repo %s from revision %s to revision %s", request.AppName, repo.Repo, syncedRevision, revision)
err := s.updateCachedRevision(logCtx, syncedRevision, revision, request, gitClientOpts)
if err != nil {
// Only warn with the error, no need to block anything if there is a caching error.
logCtx.Warnf("error updating cached revision for repo %s with revision %s: %v", repo.Repo, revision, err)
return &apiclient.UpdateRevisionForPathsResponse{}, nil
}
return &apiclient.UpdateRevisionForPathsResponse{}, nil
}
logCtx.Debugf("changes found for application %s in repo %s from revision %s to revision %s", request.AppName, repo.Repo, syncedRevision, revision)
return &apiclient.UpdateRevisionForPathsResponse{Changes: true}, nil
}
func (s *Service) updateCachedRevision(logCtx *log.Entry, oldRev string, newRev string, request *apiclient.UpdateRevisionForPathsRequest, gitClientOpts git.ClientOpts) error {
repoRefs := make(map[string]string)
if request.HasMultipleSources && request.ApplicationSource.Helm != nil {
var err error
repoRefs, err = resolveReferencedSources(true, request.ApplicationSource.Helm, request.RefSources, s.newClientResolveRevision, gitClientOpts)
if err != nil {
return fmt.Errorf("failed to get repo refs for application %s in repo %s from revision %s: %w", request.AppName, request.GetRepo().Repo, request.Revision, err)
}
}
// Update revision in refSource
if request.HasMultipleSources && request.ApplicationSource.Helm != nil {
for normalizedURL := range repoRefs {
repoRefs[normalizedURL] = newRev
}
}
err := s.cache.SetNewRevisionManifests(newRev, oldRev, request.ApplicationSource, request.RefSources, request, request.Namespace, request.TrackingMethod, request.AppLabelKey, request.AppName, repoRefs)
if err != nil {
if err == cache.ErrCacheMiss {
logCtx.Debugf("manifest cache miss during comparison for application %s in repo %s from revision %s", request.AppName, request.GetRepo().Repo, oldRev)
return nil
}
return fmt.Errorf("manifest cache move error for %s: %w", request.AppName, err)
}
logCtx.Debugf("manifest cache updated for application %s in repo %s from revision %s to revision %s", request.AppName, request.GetRepo().Repo, oldRev, newRev)
return nil
}
+26
View File
@@ -256,6 +256,28 @@ message GitDirectoriesResponse {
repeated string paths = 1;
}
message UpdateRevisionForPathsRequest {
github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.Repository repo = 1;
string appLabelKey = 2;
string appName = 3;
string namespace = 4;
github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.ApplicationSource applicationSource = 5;
string trackingMethod = 6;
map<string, github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.RefTarget> refSources = 7;
string kubeVersion = 8;
repeated string apiVersions = 9;
bool hasMultipleSources = 10;
string syncedRevision = 11;
string revision = 12;
repeated string paths = 13;
}
message UpdateRevisionForPathsResponse {
bool changes = 1;
}
// ManifestService
service RepoServerService {
@@ -310,4 +332,8 @@ service RepoServerService {
// GetGitDirectories returns a set of directory paths for the given repo
rpc GetGitDirectories(GitDirectoriesRequest) returns (GitDirectoriesResponse) {
}
// UpdateRevisionForPaths will compare two revisions and update the cache with the new revision if no changes are detected in the provided paths
rpc UpdateRevisionForPaths(UpdateRevisionForPathsRequest) returns (UpdateRevisionForPathsResponse) {
}
}
+311
View File
@@ -519,6 +519,61 @@ func TestHelmChartReferencingExternalValues(t *testing.T) {
}, response)
}
func TestHelmChartReferencingExternalValues_InvalidRefs(t *testing.T) {
spec := argoappv1.ApplicationSpec{
Sources: []argoappv1.ApplicationSource{
{RepoURL: "https://helm.example.com", Chart: "my-chart", TargetRevision: ">= 1.0.0", Helm: &argoappv1.ApplicationSourceHelm{
ValueFiles: []string{"$ref/testdata/my-chart/my-chart-values.yaml"},
}},
{RepoURL: "https://git.example.com/test/repo"},
},
}
repoDB := &dbmocks.ArgoDB{}
repoDB.On("GetRepository", context.Background(), "https://git.example.com/test/repo").Return(&argoappv1.Repository{
Repo: "https://git.example.com/test/repo",
}, nil)
// Empty refsource
service := newService(t, ".")
refSources, err := argo.GetRefSources(context.Background(), spec, repoDB)
require.NoError(t, err)
request := &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &spec.Sources[0], NoCache: true, RefSources: refSources, HasMultipleSources: true, ProjectName: "something",
ProjectSourceRepos: []string{"*"}}
response, err := service.GenerateManifest(context.Background(), request)
assert.Error(t, err)
assert.Nil(t, response)
// Invalid ref
service = newService(t, ".")
spec.Sources[1].Ref = "Invalid"
refSources, err = argo.GetRefSources(context.Background(), spec, repoDB)
require.NoError(t, err)
request = &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &spec.Sources[0], NoCache: true, RefSources: refSources, HasMultipleSources: true, ProjectName: "something",
ProjectSourceRepos: []string{"*"}}
response, err = service.GenerateManifest(context.Background(), request)
assert.Error(t, err)
assert.Nil(t, response)
// Helm chart as ref (unsupported)
service = newService(t, ".")
spec.Sources[1].Ref = "ref"
spec.Sources[1].Chart = "helm-chart"
refSources, err = argo.GetRefSources(context.Background(), spec, repoDB)
require.NoError(t, err)
request = &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &spec.Sources[0], NoCache: true, RefSources: refSources, HasMultipleSources: true, ProjectName: "something",
ProjectSourceRepos: []string{"*"}}
response, err = service.GenerateManifest(context.Background(), request)
assert.Error(t, err)
assert.Nil(t, response)
}
func TestHelmChartReferencingExternalValues_OutOfBounds_Symlink(t *testing.T) {
service := newService(t, ".")
err := os.Mkdir("testdata/oob-symlink", 0755)
@@ -3363,6 +3418,262 @@ func TestGetGitFiles(t *testing.T) {
})
}
func TestErrorUpdateRevisionForPaths(t *testing.T) {
type fields struct {
service *Service
}
type args struct {
ctx context.Context
request *apiclient.UpdateRevisionForPathsRequest
}
tests := []struct {
name string
fields fields
args args
want *apiclient.UpdateRevisionForPathsResponse
wantErr assert.ErrorAssertionFunc
}{
{name: "InvalidRepo", fields: fields{service: newService(t, ".")}, args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: nil,
Revision: "HEAD",
SyncedRevision: "sadfsadf",
},
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", mock.Anything).Return("", fmt.Errorf("ah error"))
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
}, ".")
return s
}()}, args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "not-a-valid-url"},
Revision: "sadfsadf",
SyncedRevision: "HEAD",
Paths: []string{"."},
},
}, want: nil, wantErr: assert.Error},
{name: "InvalidResolveSyncedRevision", fields: fields{service: func() *Service {
s, _, _ := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", "HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("LsRemote", mock.Anything).Return("", fmt.Errorf("ah error"))
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
}, ".")
return s
}()}, args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "not-a-valid-url"},
Revision: "HEAD",
SyncedRevision: "sadfsadf",
Paths: []string{"."},
},
}, want: nil, wantErr: assert.Error},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := tt.fields.service
got, err := s.UpdateRevisionForPaths(tt.args.ctx, tt.args.request)
if !tt.wantErr(t, err, fmt.Sprintf("UpdateRevisionForPaths(%v, %v)", tt.args.ctx, tt.args.request)) {
return
}
assert.Equalf(t, tt.want, got, "UpdateRevisionForPaths(%v, %v)", tt.args.ctx, tt.args.request)
})
}
}
func TestUpdateRevisionForPaths(t *testing.T) {
type fields struct {
service *Service
cache *repoCacheMocks
}
type args struct {
ctx context.Context
request *apiclient.UpdateRevisionForPathsRequest
}
type cacheHit struct {
revision string
previousRevision string
}
tests := []struct {
name string
fields fields
args args
want *apiclient.UpdateRevisionForPathsResponse
wantErr assert.ErrorAssertionFunc
cacheHit *cacheHit
}{
{name: "NoPathAbort", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
}, ".")
return fields{
service: s,
cache: c,
}
}(), args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "a-url.com"},
Paths: []string{},
},
}, want: &apiclient.UpdateRevisionForPathsResponse{}, wantErr: assert.NoError},
{name: "SameResolvedRevisionAbort", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", "HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("LsRemote", "SYNCEDHEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
}, ".")
return fields{
service: s,
cache: c,
}
}(), args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "a-url.com"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
},
}, want: &apiclient.UpdateRevisionForPathsResponse{}, wantErr: assert.NoError},
{name: "ChangedFilesDoNothing", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Init").Return(nil)
gitClient.On("Fetch", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", "HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("LsRemote", "SYNCEDHEAD").Once().Return("1e67a504d03def3a6a1125d934cb511680f72555", nil)
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
gitClient.On("Root").Return("")
gitClient.On("ChangedFiles", mock.Anything, mock.Anything).Return([]string{"app.yaml"}, nil)
}, ".")
return fields{
service: s,
cache: c,
}
}(), args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "a-url.com"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
},
}, want: &apiclient.UpdateRevisionForPathsResponse{
Changes: true,
}, wantErr: assert.NoError},
{name: "NoChangesUpdateCache", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Init").Return(nil)
gitClient.On("Fetch", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", "HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("LsRemote", "SYNCEDHEAD").Once().Return("1e67a504d03def3a6a1125d934cb511680f72555", nil)
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
gitClient.On("Root").Return("")
gitClient.On("ChangedFiles", mock.Anything, mock.Anything).Return([]string{}, nil)
}, ".")
return fields{
service: s,
cache: c,
}
}(), args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "a-url.com"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
AppLabelKey: "app.kubernetes.io/name",
AppName: "no-change-update-cache",
Namespace: "default",
TrackingMethod: "annotation+label",
ApplicationSource: &argoappv1.ApplicationSource{Path: "."},
KubeVersion: "v1.16.0",
},
}, want: &apiclient.UpdateRevisionForPathsResponse{}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "1e67a504d03def3a6a1125d934cb511680f72555",
revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}},
{name: "NoChangesHelmMultiSourceUpdateCache", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, helmClient *helmmocks.Client, paths *iomocks.TempPaths) {
gitClient.On("Init").Return(nil)
gitClient.On("Fetch", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", "HEAD").Once().Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("LsRemote", "SYNCEDHEAD").Once().Return("1e67a504d03def3a6a1125d934cb511680f72555", nil)
paths.On("GetPath", mock.Anything).Return(".", nil)
paths.On("GetPathIfExists", mock.Anything).Return(".", nil)
gitClient.On("Root").Return("")
gitClient.On("ChangedFiles", mock.Anything, mock.Anything).Return([]string{}, nil)
}, ".")
return fields{
service: s,
cache: c,
}
}(), args: args{
ctx: context.TODO(),
request: &apiclient.UpdateRevisionForPathsRequest{
Repo: &argoappv1.Repository{Repo: "a-url.com"},
Revision: "HEAD",
SyncedRevision: "SYNCEDHEAD",
Paths: []string{"."},
AppLabelKey: "app.kubernetes.io/name",
AppName: "no-change-update-cache",
Namespace: "default",
TrackingMethod: "annotation+label",
ApplicationSource: &argoappv1.ApplicationSource{Path: ".", Helm: &argoappv1.ApplicationSourceHelm{ReleaseName: "test"}},
KubeVersion: "v1.16.0",
HasMultipleSources: true,
},
}, want: &apiclient.UpdateRevisionForPathsResponse{}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "1e67a504d03def3a6a1125d934cb511680f72555",
revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := tt.fields.service
cache := tt.fields.cache
if tt.cacheHit != nil {
cache.mockCache.On("Rename", tt.cacheHit.previousRevision, tt.cacheHit.revision, mock.Anything).Return(nil)
}
got, err := s.UpdateRevisionForPaths(tt.args.ctx, tt.args.request)
if !tt.wantErr(t, err, fmt.Sprintf("UpdateRevisionForPaths(%v, %v)", tt.args.ctx, tt.args.request)) {
return
}
assert.Equalf(t, tt.want, got, "UpdateRevisionForPaths(%v, %v)", tt.args.ctx, tt.args.request)
if tt.cacheHit != nil {
cache.mockCache.AssertCacheCalledTimes(t, &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
})
} else {
cache.mockCache.AssertCacheCalledTimes(t, &repositorymocks.CacheCallCounts{
ExternalRenames: 0,
})
}
})
}
}
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>")
+2
View File
@@ -1,6 +1,7 @@
package e2e
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
@@ -73,6 +74,7 @@ func TestMultiSourceAppWithHelmExternalValueFiles(t *testing.T) {
},
},
}}
fmt.Printf("sources: %v\n", sources)
ctx := Given(t)
ctx.
Sources(sources).
+64
View File
@@ -6,7 +6,9 @@ import (
"path/filepath"
"strings"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/util/io/files"
"github.com/argoproj/argo-cd/v2/util/security"
)
func Path(root, path string) (string, error) {
@@ -88,3 +90,65 @@ func CheckOutOfBoundsSymlinks(basePath string) error {
return nil
})
}
// GetAppRefreshPaths returns the list of paths that should trigger a refresh for an application
func GetAppRefreshPaths(app *v1alpha1.Application) []string {
var paths []string
if val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]; ok && val != "" {
for _, item := range strings.Split(val, ";") {
if item == "" {
continue
}
if filepath.IsAbs(item) {
paths = append(paths, item[1:])
} else {
for _, source := range app.Spec.GetSources() {
paths = append(paths, filepath.Clean(filepath.Join(source.Path, item)))
}
}
}
}
return paths
}
// AppFilesHaveChanged returns true if any of the changed files are under the given refresh paths
// If refreshPaths is empty, it will always return true
func AppFilesHaveChanged(refreshPaths []string, changedFiles []string) bool {
// empty slice means there was no changes to any files
// so we should not refresh
if len(changedFiles) == 0 {
return false
}
if len(refreshPaths) == 0 {
// Apps without a given refreshed paths always be refreshed, regardless of changed files
// this is the "default" behavior
return true
}
// At last one changed file must be under refresh path
for _, f := range changedFiles {
f = ensureAbsPath(f)
for _, item := range refreshPaths {
item = ensureAbsPath(item)
changed := false
if f == item {
changed = true
} else if _, err := security.EnforceToCurrentRoot(item, f); err == nil {
changed = true
}
if changed {
return true
}
}
}
return false
}
func ensureAbsPath(input string) string {
if !filepath.IsAbs(input) {
return string(filepath.Separator) + input
}
return input
}
+113
View File
@@ -8,7 +8,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
fileutil "github.com/argoproj/argo-cd/v2/test/fixture/path"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func TestPathRoot(t *testing.T) {
@@ -90,3 +92,114 @@ func TestAbsSymlink(t *testing.T) {
assert.ErrorAs(t, err, &oobError)
assert.Equal(t, oobError.File, "abslink")
}
func getApp(annotation string, sourcePath string) *v1alpha1.Application {
return &v1alpha1.Application{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
},
},
Spec: v1alpha1.ApplicationSpec{
Source: &v1alpha1.ApplicationSource{
Path: sourcePath,
},
},
}
}
func getMultiSourceApp(annotation string, paths ...string) *v1alpha1.Application {
var sources v1alpha1.ApplicationSources
for _, path := range paths {
sources = append(sources, v1alpha1.ApplicationSource{Path: path})
}
return &v1alpha1.Application{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
},
},
Spec: v1alpha1.ApplicationSpec{
Sources: sources,
},
}
}
func Test_AppFilesHaveChanged(t *testing.T) {
tests := []struct {
name string
app *v1alpha1.Application
files []string
changeExpected bool
}{
{"default no path", &v1alpha1.Application{}, []string{"README.md"}, true},
{"no files changed", getApp(".", "source/path"), []string{}, false},
{"relative path - matching", getApp(".", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path, multi source - matching #1", getMultiSourceApp(".", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path, multi source - matching #2", getMultiSourceApp(".", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path - not matching", getApp(".", "source/path"), []string{"README.md"}, false},
{"relative path, multi source - not matching", getMultiSourceApp(".", "other/path", "unrelated/path"), []string{"README.md"}, false},
{"absolute path - matching", getApp("/source/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path, multi source - matching #1", getMultiSourceApp("/source/path", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path, multi source - matching #2", getMultiSourceApp("/source/path", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path - not matching", getApp("/source/path1", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"absolute path, multi source - not matching", getMultiSourceApp("/source/path1", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"two relative paths - matching", getApp(".;../shared", "my-app"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths, multi source - matching #1", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths, multi source - matching #2", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths - not matching", getApp(".;../shared", "my-app"), []string{"README.md"}, false},
{"two relative paths, multi source - not matching", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"README.md"}, false},
{"file relative path - matching", getApp("./my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path, multi source - matching #1", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path, multi source - matching #2", getMultiSourceApp("./my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path - not matching", getApp("./my-deployment.yaml", "source/path"), []string{"README.md"}, false},
{"file relative path, multi source - not matching", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"README.md"}, false},
{"file absolute path - matching", getApp("/source/path/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path, multi source - matching #1", getMultiSourceApp("/source/path/my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path, multi source - matching #2", getMultiSourceApp("/source/path/my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path - not matching", getApp("/source/path1/README.md", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"file absolute path, multi source - not matching", getMultiSourceApp("/source/path1/README.md", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, false},
{"file two relative paths - matching", getApp("./README.md;../shared/my-deployment.yaml", "my-app"), []string{"shared/my-deployment.yaml"}, true},
{"file two relative paths, multi source - matching", getMultiSourceApp("./README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"shared/my-deployment.yaml"}, true},
{"file two relative paths - not matching", getApp(".README.md;../shared/my-deployment.yaml", "my-app"), []string{"kustomization.yaml"}, false},
{"file two relative paths, multi source - not matching", getMultiSourceApp(".README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"kustomization.yaml"}, false},
{"changed file absolute path - matching", getApp(".", "source/path"), []string{"/source/path/my-deployment.yaml"}, true},
}
for _, tt := range tests {
ttc := tt
t.Run(ttc.name, func(t *testing.T) {
t.Parallel()
refreshPaths := GetAppRefreshPaths(ttc.app)
if got := AppFilesHaveChanged(refreshPaths, ttc.files); got != ttc.changeExpected {
t.Errorf("AppFilesHaveChanged() = %v, want %v", got, ttc.changeExpected)
}
})
}
}
func Test_GetAppRefreshPaths(t *testing.T) {
tests := []struct {
name string
app *v1alpha1.Application
expectedPaths []string
}{
{"default no path", &v1alpha1.Application{}, []string{}},
{"relative path", getApp(".", "source/path"), []string{"source/path"}},
{"absolute path", getApp("/source/path", "source/path"), []string{"source/path"}},
{"absolute path - multi source", getMultiSourceApp("/source/path", "source/path", "other/path"), []string{"source/path"}},
{"two relative paths ", getApp(".;../shared", "my-app"), []string{"my-app", "shared"}},
{"file relative path", getApp("./my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}},
{"file absolute path", getApp("/source/path/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}},
{"file two relative paths", getApp("./README.md;../shared/my-deployment.yaml", "my-app"), []string{"my-app/README.md", "shared/my-deployment.yaml"}},
{"empty path", getApp(".;", "source/path"), []string{"source/path"}},
}
for _, tt := range tests {
ttc := tt
t.Run(ttc.name, func(t *testing.T) {
t.Parallel()
if got := GetAppRefreshPaths(ttc.app); !assert.ElementsMatch(t, ttc.expectedPaths, got) {
t.Errorf("GetAppRefreshPath() = %v, want %v", got, ttc.expectedPaths)
}
})
}
}
+6 -1
View File
@@ -97,7 +97,12 @@ func (r *redisCache) unmarshal(data []byte, obj interface{}) error {
}
func (r *redisCache) Rename(oldKey string, newKey string, _ time.Duration) error {
return r.client.Rename(context.TODO(), r.getKey(oldKey), r.getKey(newKey)).Err()
err := r.client.Rename(context.TODO(), r.getKey(oldKey), r.getKey(newKey)).Err()
if err != nil && err.Error() == "ERR no such key" {
err = ErrCacheMiss
}
return err
}
func (r *redisCache) Set(item *Item) error {
+24
View File
@@ -75,6 +75,7 @@ type Client interface {
RevisionMetadata(revision string) (*RevisionMetadata, error)
VerifyCommitSignature(string) (string, error)
IsAnnotatedTag(string) bool
ChangedFiles(revision string, targetRevision string) ([]string, error)
}
type EventHandlers struct {
@@ -704,6 +705,29 @@ func (m *nativeGitClient) IsAnnotatedTag(revision string) bool {
}
}
// returns the meta-data for the commit
func (m *nativeGitClient) ChangedFiles(revision string, targetRevision string) ([]string, error) {
if revision == targetRevision {
return []string{}, nil
}
if !IsCommitSHA(revision) || !IsCommitSHA(targetRevision) {
return []string{}, fmt.Errorf("invalid revision provided, must be SHA")
}
out, err := m.runCmd("diff", "--name-only", fmt.Sprintf("%s..%s", revision, targetRevision))
if err != nil {
return nil, fmt.Errorf("failed to diff %s..%s: %w", revision, targetRevision, err)
}
if out == "" {
return []string{}, nil
}
files := strings.Split(out, "\n")
return files, nil
}
// runWrapper runs a custom command with all the semantics of running the Git client
func (m *nativeGitClient) runGnuPGWrapper(wrapper string, args ...string) (string, error) {
cmd := exec.Command(wrapper, args...)
+55
View File
@@ -118,6 +118,61 @@ func Test_IsAnnotatedTag(t *testing.T) {
assert.False(t, atag)
}
func Test_ChangedFiles(t *testing.T) {
tempDir := t.TempDir()
client, err := NewClientExt(fmt.Sprintf("file://%s", tempDir), tempDir, NopCreds{}, true, false, "")
require.NoError(t, err)
err = client.Init()
require.NoError(t, err)
err = runCmd(client.Root(), "git", "commit", "-m", "Initial commit", "--allow-empty")
require.NoError(t, err)
// Create a tag to have a second ref
err = runCmd(client.Root(), "git", "tag", "some-tag")
require.NoError(t, err)
p := path.Join(client.Root(), "README")
f, err := os.Create(p)
require.NoError(t, err)
_, err = f.WriteString("Hello.")
require.NoError(t, err)
err = f.Close()
require.NoError(t, err)
err = runCmd(client.Root(), "git", "add", "README")
require.NoError(t, err)
err = runCmd(client.Root(), "git", "commit", "-m", "Changes", "-a")
require.NoError(t, err)
previousSHA, err := client.LsRemote("some-tag")
require.NoError(t, err)
commitSHA, err := client.LsRemote("HEAD")
require.NoError(t, err)
// Invalid commits, error
_, err = client.ChangedFiles("0000000000000000000000000000000000000000", "1111111111111111111111111111111111111111")
require.Error(t, err)
// Not SHAs, error
_, err = client.ChangedFiles(previousSHA, "HEAD")
require.Error(t, err)
// Same commit, no changes
changedFiles, err := client.ChangedFiles(commitSHA, commitSHA)
require.NoError(t, err)
assert.ElementsMatch(t, []string{}, changedFiles)
// Different ref, with changes
changedFiles, err = client.ChangedFiles(previousSHA, commitSHA)
require.NoError(t, err)
assert.ElementsMatch(t, []string{"README"}, changedFiles)
}
func Test_nativeGitClient_Submodule(t *testing.T) {
tempDir, err := os.MkdirTemp("", "")
require.NoError(t, err)
+27 -1
View File
@@ -1,4 +1,4 @@
// Code generated by mockery v2.30.1. DO NOT EDIT.
// Code generated by mockery v2.32.4. DO NOT EDIT.
package mocks
@@ -12,6 +12,32 @@ type Client struct {
mock.Mock
}
// ChangedFiles provides a mock function with given fields: revision, targetRevision
func (_m *Client) ChangedFiles(revision string, targetRevision string) ([]string, error) {
ret := _m.Called(revision, targetRevision)
var r0 []string
var r1 error
if rf, ok := ret.Get(0).(func(string, string) ([]string, error)); ok {
return rf(revision, targetRevision)
}
if rf, ok := ret.Get(0).(func(string, string) []string); ok {
r0 = rf(revision, targetRevision)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]string)
}
}
if rf, ok := ret.Get(1).(func(string, string) error); ok {
r1 = rf(revision, targetRevision)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// Checkout provides a mock function with given fields: revision, submoduleEnabled
func (_m *Client) Checkout(revision string, submoduleEnabled bool) error {
ret := _m.Called(revision, submoduleEnabled)
+3 -67
View File
@@ -7,7 +7,6 @@ import (
"html"
"net/http"
"net/url"
"path/filepath"
"regexp"
"strings"
@@ -26,10 +25,10 @@ import (
appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
"github.com/argoproj/argo-cd/v2/reposerver/cache"
servercache "github.com/argoproj/argo-cd/v2/server/cache"
"github.com/argoproj/argo-cd/v2/util/app/path"
"github.com/argoproj/argo-cd/v2/util/argo"
"github.com/argoproj/argo-cd/v2/util/db"
"github.com/argoproj/argo-cd/v2/util/glob"
"github.com/argoproj/argo-cd/v2/util/security"
"github.com/argoproj/argo-cd/v2/util/settings"
)
@@ -292,7 +291,8 @@ func (a *ArgoCDWebhookHandler) HandleEvent(payload interface{}) {
for _, source := range app.Spec.GetSources() {
if sourceRevisionHasChanged(source, revision, touchedHead) && sourceUsesURL(source, webURL, repoRegexp) {
if appFilesHaveChanged(&app, changedFiles) {
refreshPaths := path.GetAppRefreshPaths(&app)
if path.AppFilesHaveChanged(refreshPaths, changedFiles) {
namespacedAppInterface := a.appClientset.ArgoprojV1alpha1().Applications(app.ObjectMeta.Namespace)
_, err = argo.RefreshApp(namespacedAppInterface, app.ObjectMeta.Name, v1alpha1.RefreshTypeNormal)
if err != nil {
@@ -358,70 +358,6 @@ func (a *ArgoCDWebhookHandler) storePreviouslyCachedManifests(app *v1alpha1.Appl
return nil
}
func getAppRefreshPaths(app *v1alpha1.Application) []string {
var paths []string
if val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]; ok && val != "" {
for _, item := range strings.Split(val, ";") {
if item == "" {
continue
}
if filepath.IsAbs(item) {
paths = append(paths, item[1:])
} else {
for _, source := range app.Spec.GetSources() {
paths = append(paths, filepath.Clean(filepath.Join(source.Path, item)))
}
}
}
}
return paths
}
func appFilesHaveChanged(app *v1alpha1.Application, changedFiles []string) bool {
// an empty slice of changed files means that the payload didn't include a list
// of changed files and w have to assume that a refresh is required
if len(changedFiles) == 0 {
return true
}
// Check to see if the app has requested refreshes only on a specific prefix
refreshPaths := getAppRefreshPaths(app)
if len(refreshPaths) == 0 {
// Apps without a given refreshed paths always be refreshed, regardless of changed files
// this is the "default" behavior
return true
}
// At last one changed file must be under refresh path
for _, f := range changedFiles {
f = ensureAbsPath(f)
for _, item := range refreshPaths {
item = ensureAbsPath(item)
changed := false
if f == item {
changed = true
} else if _, err := security.EnforceToCurrentRoot(item, f); err == nil {
changed = true
}
if changed {
log.WithField("application", app.Name).Debugf("Application uses files that have changed")
return true
}
}
}
log.WithField("application", app.Name).Debugf("Application does not use any of the files that have changed")
return false
}
func ensureAbsPath(input string) string {
if !filepath.IsAbs(input) {
return string(filepath.Separator) + input
}
return input
}
func sourceRevisionHasChanged(source v1alpha1.ApplicationSource, revision string, touchedHead bool) bool {
targetRev := parseRevision(source.TargetRevision)
if targetRev == "HEAD" || targetRev == "" { // revision is head
-81
View File
@@ -411,87 +411,6 @@ func TestUnknownEvent(t *testing.T) {
hook.Reset()
}
func getApp(annotation string, sourcePath string) *v1alpha1.Application {
return &v1alpha1.Application{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
},
},
Spec: v1alpha1.ApplicationSpec{
Source: &v1alpha1.ApplicationSource{
Path: sourcePath,
},
},
}
}
func getMultiSourceApp(annotation string, paths ...string) *v1alpha1.Application {
var sources v1alpha1.ApplicationSources
for _, path := range paths {
sources = append(sources, v1alpha1.ApplicationSource{Path: path})
}
return &v1alpha1.Application{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
},
},
Spec: v1alpha1.ApplicationSpec{
Sources: sources,
},
}
}
func Test_getAppRefreshPrefix(t *testing.T) {
tests := []struct {
name string
app *v1alpha1.Application
files []string
changeExpected bool
}{
{"default no path", &v1alpha1.Application{}, []string{"README.md"}, true},
{"relative path - matching", getApp(".", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path, multi source - matching #1", getMultiSourceApp(".", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path, multi source - matching #2", getMultiSourceApp(".", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"relative path - not matching", getApp(".", "source/path"), []string{"README.md"}, false},
{"relative path, multi source - not matching", getMultiSourceApp(".", "other/path", "unrelated/path"), []string{"README.md"}, false},
{"absolute path - matching", getApp("/source/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path, multi source - matching #1", getMultiSourceApp("/source/path", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path, multi source - matching #2", getMultiSourceApp("/source/path", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"absolute path - not matching", getApp("/source/path1", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"absolute path, multi source - not matching", getMultiSourceApp("/source/path1", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"two relative paths - matching", getApp(".;../shared", "my-app"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths, multi source - matching #1", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths, multi source - matching #2", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
{"two relative paths - not matching", getApp(".;../shared", "my-app"), []string{"README.md"}, false},
{"two relative paths, multi source - not matching", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"README.md"}, false},
{"file relative path - matching", getApp("./my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path, multi source - matching #1", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path, multi source - matching #2", getMultiSourceApp("./my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file relative path - not matching", getApp("./my-deployment.yaml", "source/path"), []string{"README.md"}, false},
{"file relative path, multi source - not matching", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"README.md"}, false},
{"file absolute path - matching", getApp("/source/path/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path, multi source - matching #1", getMultiSourceApp("/source/path/my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path, multi source - matching #2", getMultiSourceApp("/source/path/my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
{"file absolute path - not matching", getApp("/source/path1/README.md", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
{"file absolute path, multi source - not matching", getMultiSourceApp("/source/path1/README.md", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, false},
{"file two relative paths - matching", getApp("./README.md;../shared/my-deployment.yaml", "my-app"), []string{"shared/my-deployment.yaml"}, true},
{"file two relative paths, multi source - matching", getMultiSourceApp("./README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"shared/my-deployment.yaml"}, true},
{"file two relative paths - not matching", getApp(".README.md;../shared/my-deployment.yaml", "my-app"), []string{"kustomization.yaml"}, false},
{"file two relative paths, multi source - not matching", getMultiSourceApp(".README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"kustomization.yaml"}, false},
}
for _, tt := range tests {
ttc := tt
t.Run(ttc.name, func(t *testing.T) {
t.Parallel()
if got := appFilesHaveChanged(ttc.app, ttc.files); got != ttc.changeExpected {
t.Errorf("getAppRefreshPrefix() = %v, want %v", got, ttc.changeExpected)
}
})
}
}
func TestAppRevisionHasChanged(t *testing.T) {
getSource := func(targetRevision string) v1alpha1.ApplicationSource {
return v1alpha1.ApplicationSource{TargetRevision: targetRevision}