Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(appset): ignoreApplicationDifferences (#9101) #14743

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package controllers

import (
"context"
"encoding/json"
"fmt"
"reflect"
"time"
Expand All @@ -24,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
apierr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
Expand All @@ -44,6 +46,7 @@ import (
"github.com/argoproj/argo-cd/v2/applicationset/generators"
"github.com/argoproj/argo-cd/v2/applicationset/utils"
"github.com/argoproj/argo-cd/v2/common"
argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff"
"github.com/argoproj/argo-cd/v2/util/db"
"github.com/argoproj/argo-cd/v2/util/glob"

Expand Down Expand Up @@ -683,6 +686,14 @@ func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context,

found.ObjectMeta.Finalizers = generatedApp.Finalizers
found.ObjectMeta.Labels = generatedApp.Labels

if found != nil && len(found.Spec.IgnoreDifferences) > 0 {
err := applyIgnoreDifferences(applicationSet.Spec.IgnoreApplicationDifferences, found, generatedApp)
if err != nil {
return fmt.Errorf("failed to apply ignore differences: %w", err)
}
}

return controllerutil.SetControllerReference(&applicationSet, found, r.Scheme)
})

Expand All @@ -700,6 +711,54 @@ func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context,
return firstError
}

// applyIgnoreDifferences applies the ignore differences rules to the found application. It modifies the found application in place.
func applyIgnoreDifferences(applicationSetIgnoreDifferences argov1alpha1.ApplicationSetIgnoreDifferences, found *argov1alpha1.Application, generatedApp argov1alpha1.Application) error {
diffConfig, err := argodiff.NewDiffConfigBuilder().
WithDiffSettings(applicationSetIgnoreDifferences.ToApplicationIgnoreDifferences(), nil, false).
WithNoCache().
Build()
if err != nil {
return fmt.Errorf("failed to build diff config: %w", err)
}
unstructuredFound, err := appToUnstructured(found)
if err != nil {
return fmt.Errorf("failed to convert found application to unstructured: %w", err)
}
unstructuredGenerated, err := appToUnstructured(&generatedApp)
if err != nil {
return fmt.Errorf("failed to convert found application to unstructured: %w", err)
}
result, err := argodiff.Normalize([]*unstructured.Unstructured{unstructuredFound}, []*unstructured.Unstructured{unstructuredGenerated}, diffConfig)
if err != nil {
return fmt.Errorf("failed to normalize application spec: %w", err)
}
if len(result.Targets) != 1 {
return fmt.Errorf("expected 1 normalized application, got %d", len(result.Targets))
}
jsonNormalized, err := json.Marshal(result.Targets[0].Object)
if err != nil {
return fmt.Errorf("failed to marshal normalized app to json: %w", err)
}
err = json.Unmarshal(jsonNormalized, &found)
if err != nil {
return fmt.Errorf("failed to unmarshal normalized app json to structured app: %w", err)
}
// Prohibit jq queries from mutating silly things.
found.TypeMeta = generatedApp.TypeMeta
found.Name = generatedApp.Name
found.Namespace = generatedApp.Namespace
found.Operation = generatedApp.Operation
return nil
}

func appToUnstructured(app *argov1alpha1.Application) (*unstructured.Unstructured, error) {
u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(app)
if err != nil {
return nil, fmt.Errorf("failed to convert app object to unstructured: %w", err)
}
return &unstructured.Unstructured{Object: u}, nil
}

// createInCluster will filter from the desiredApplications only the application that needs to be created
// Then it will call createOrUpdateInCluster to do the actual create
func (r *ApplicationSetReconciler) createInCluster(ctx context.Context, applicationSet argov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) error {
Expand Down
172 changes: 172 additions & 0 deletions applicationset/controllers/applicationset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -5726,3 +5728,173 @@ func TestOwnsHandler(t *testing.T) {
})
}
}

func Test_applyIgnoreDifferences(t *testing.T) {
appMeta := metav1.TypeMeta{
APIVersion: v1alpha1.ApplicationSchemaGroupVersionKind.GroupVersion().String(),
Kind: v1alpha1.ApplicationSchemaGroupVersionKind.Kind,
}
testCases := []struct {
name string
ignoreDifferences v1alpha1.ApplicationSetIgnoreDifferences
foundApp string
generatedApp string
expectedApp string
}{
{
name: "empty ignoreDifferences",
foundApp: `
spec: {}`,
generatedApp: `
spec: {}`,
expectedApp: `
spec: {}`,
},
{
// For this use case: https://github.com/argoproj/argo-cd/issues/9101#issuecomment-1191138278
name: "ignore target revision with jq",
ignoreDifferences: v1alpha1.ApplicationSetIgnoreDifferences{
{JQPathExpressions: []string{".spec.source.targetRevision"}},
},
foundApp: `
spec:
source:
targetRevision: foo`,
generatedApp: `
spec:
source:
targetRevision: bar`,
expectedApp: `
spec:
source:
targetRevision: foo`,
},
{
// For this use case: https://github.com/argoproj/argo-cd/issues/9101#issuecomment-1103593714
name: "ignore helm parameter with jq",
ignoreDifferences: v1alpha1.ApplicationSetIgnoreDifferences{
{JQPathExpressions: []string{`.spec.source.helm.parameters | select(.name == "image.tag")`}},
},
foundApp: `
spec:
source:
helm:
parameters:
- name: image.tag
value: test
- name: another
value: value`,
generatedApp: `
spec:
source:
helm:
parameters:
- name: image.tag
value: v1.0.0
- name: another
value: value`,
expectedApp: `
spec:
source:
helm:
parameters:
- name: image.tag
value: test
- name: another
value: value`,
},
{
// For this use case: https://github.com/argoproj/argo-cd/issues/9101#issuecomment-1191138278
name: "ignore auto-sync with jq",
ignoreDifferences: v1alpha1.ApplicationSetIgnoreDifferences{
{JQPathExpressions: []string{".spec.syncPolicy.automated"}},
},
foundApp: `
spec:
syncPolicy:
retry:
limit: 5`,
generatedApp: `
spec:
syncPolicy:
automated:
selfHeal: true
retry:
limit: 5`,
expectedApp: `
spec:
syncPolicy:
retry:
limit: 5`,
},
{
// For this use case: https://github.com/argoproj/argo-cd/issues/9101#issuecomment-1420656537
name: "ignore a one-off annotation with jq",
ignoreDifferences: v1alpha1.ApplicationSetIgnoreDifferences{
{JQPathExpressions: []string{`.metadata.annotations | select(.["foo.bar"] == "baz")`}},
},
foundApp: `
metadata:
annotations:
foo.bar: baz
some.other: annotation`,
generatedApp: `
metadata:
annotations:
some.other: annotation`,
expectedApp: `
metadata:
annotations:
foo.bar: baz
some.other: annotation`,
},
{
// For this use case: https://github.com/argoproj/argo-cd/issues/9101#issuecomment-1515672638
name: "ignore the source.plugin field with a json pointer",
ignoreDifferences: v1alpha1.ApplicationSetIgnoreDifferences{
{JSONPointers: []string{"/spec/source/plugin"}},
},
foundApp: `
spec:
source:
plugin:
parameters:
- name: url
string: https://example.com`,
generatedApp: `
spec:
source:
plugin:
parameters:
- name: url
string: https://example.com/wrong`,
expectedApp: `
spec:
source:
plugin:
parameters:
- name: url
string: https://example.com`,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
foundApp := v1alpha1.Application{TypeMeta: appMeta}
err := yaml.Unmarshal([]byte(tc.foundApp), &foundApp)
require.NoError(t, err, tc.foundApp)
generatedApp := v1alpha1.Application{TypeMeta: appMeta}
err = yaml.Unmarshal([]byte(tc.generatedApp), &generatedApp)
require.NoError(t, err, tc.generatedApp)
err = applyIgnoreDifferences(tc.ignoreDifferences, &foundApp, generatedApp)
require.NoError(t, err)
jsonFound, err := json.Marshal(tc.foundApp)
require.NoError(t, err)
jsonExpected, err := json.Marshal(tc.expectedApp)
require.NoError(t, err)
assert.Equal(t, string(jsonExpected), string(jsonFound))
})
}
}
13 changes: 6 additions & 7 deletions applicationset/utils/clusterUtils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
kubetesting "k8s.io/client-go/testing"

argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/test/e2e/fixture/applicationsets/utils"
)

const (
Expand Down Expand Up @@ -69,7 +68,7 @@ func createClusterSecret(secretName string, clusterName string, clusterServer st
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: utils.ArgoCDNamespace,
Namespace: fakeNamespace,
Labels: map[string]string{
ArgoCDSecretTypeLabel: ArgoCDSecretTypeCluster,
},
Expand Down Expand Up @@ -111,7 +110,7 @@ func TestValidateDestination(t *testing.T) {
objects = append(objects, secret)
kubeclientset := fake.NewSimpleClientset(objects...)

appCond := ValidateDestination(context.Background(), &dest, kubeclientset, utils.ArgoCDNamespace)
appCond := ValidateDestination(context.Background(), &dest, kubeclientset, fakeNamespace)
assert.Nil(t, appCond)
assert.Equal(t, "https://127.0.0.1:6443", dest.Server)
assert.True(t, dest.IsServerInferred())
Expand All @@ -124,7 +123,7 @@ func TestValidateDestination(t *testing.T) {
Namespace: "default",
}

err := ValidateDestination(context.Background(), &dest, nil, utils.ArgoCDNamespace)
err := ValidateDestination(context.Background(), &dest, nil, fakeNamespace)
assert.Equal(t, "application destination can't have both name and server defined: minikube https://127.0.0.1:6443", err.Error())
assert.False(t, dest.IsServerInferred())
})
Expand All @@ -139,7 +138,7 @@ func TestValidateDestination(t *testing.T) {
return true, nil, fmt.Errorf("an error occurred")
})

err := ValidateDestination(context.Background(), &dest, kubeclientset, utils.ArgoCDNamespace)
err := ValidateDestination(context.Background(), &dest, kubeclientset, fakeNamespace)
assert.Equal(t, "unable to find destination server: an error occurred", err.Error())
assert.False(t, dest.IsServerInferred())
})
Expand All @@ -154,7 +153,7 @@ func TestValidateDestination(t *testing.T) {
objects = append(objects, secret)
kubeclientset := fake.NewSimpleClientset(objects...)

err := ValidateDestination(context.Background(), &dest, kubeclientset, utils.ArgoCDNamespace)
err := ValidateDestination(context.Background(), &dest, kubeclientset, fakeNamespace)
assert.Equal(t, "unable to find destination server: there are no clusters with this name: minikube", err.Error())
assert.False(t, dest.IsServerInferred())
})
Expand All @@ -171,7 +170,7 @@ func TestValidateDestination(t *testing.T) {
objects = append(objects, secret, secret2)
kubeclientset := fake.NewSimpleClientset(objects...)

err := ValidateDestination(context.Background(), &dest, kubeclientset, utils.ArgoCDNamespace)
err := ValidateDestination(context.Background(), &dest, kubeclientset, fakeNamespace)
assert.Equal(t, "unable to find destination server: there are 2 clusters with the same name: [https://127.0.0.1:2443 https://127.0.0.1:8443]", err.Error())
assert.False(t, dest.IsServerInferred())
})
Expand Down
30 changes: 30 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -6055,6 +6055,30 @@
}
}
},
"v1alpha1ApplicationSetResourceIgnoreDifferences": {
"description": "ApplicationSetResourceIgnoreDifferences configures how the ApplicationSet controller will ignore differences in live\napplications when applying changes from generated applications.",
"type": "object",
"properties": {
"jqPathExpressions": {
"description": "JQPathExpressions is a list of JQ path expressions to fields to ignore differences for.",
"type": "array",
"items": {
"type": "string"
}
},
"jsonPointers": {
"description": "JSONPointers is a list of JSON pointers to fields to ignore differences for.",
"type": "array",
"items": {
"type": "string"
}
},
"name": {
"description": "Name is the name of the application to ignore differences for. If not specified, the rule applies to all applications.",
"type": "string"
}
}
},
"v1alpha1ApplicationSetRolloutStep": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -6103,6 +6127,12 @@
"type": "string"
}
},
"ignoreApplicationDifferences": {
"type": "array",
"items": {
"$ref": "#/definitions/v1alpha1ApplicationSetResourceIgnoreDifferences"
}
},
"preservedFields": {
"$ref": "#/definitions/v1alpha1ApplicationPreservedFields"
},
Expand Down
10 changes: 10 additions & 0 deletions docs/operator-manual/applicationset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,13 @@ spec:
preserveResourcesOnDeletion: false
# Alpha feature to determine the order in which ApplicationSet applies changes.
strategy:
# This field lets you define fields which should be ignored when applying Application resources. This is helpful if you
# want to use ApplicationSets to create apps, but also want to allow users to modify those apps without having their
# changes overwritten by the ApplicationSet.
ignoreApplicationDifferences:
- jsonPointers:
- /spec/source/targetRevision
- name: some-app
jqExpressions:
- .spec.source.helm.values

Loading
Loading