marcel/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd.git synced 2026-02-20 01:28:45 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: revert #24197 (#25294)

Browse Source 
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
This commit is contained in:
Blake Pettersson
2025-11-17 20:21:44 +01:00
committed by GitHub
parent c79f17167c
commit 5444415c86
11 changed files with 28 additions and 509 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
controller/sync.go
View File

@@ -2,7 +2,6 @@ package controller
import (
"context"
"encoding/json"
stderrors "errors"
"fmt"
"os"
@@ -263,7 +262,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, project *v1alp
// resources which in this case applies the live values in the configured
// ignore differences fields.
if syncOp.SyncOptions.HasOption("RespectIgnoreDifferences=true") {
patchedTargets, err := normalizeTargetResources(openAPISchema, compareResult)
patchedTargets, err := normalizeTargetResources(compareResult)
if err != nil {
state.Phase = common.OperationError
state.Message = fmt.Sprintf("Failed to normalize target resources: %s", err)
@@ -435,65 +434,53 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, project *v1alp
// - applies normalization to the target resources based on the live resources
// - copies ignored fields from the matching live resources: apply normalizer to the live resource,
// calculates the patch performed by normalizer and applies the patch to the target resource
func normalizeTargetResources(openAPISchema openapi.Resources, cr *comparisonResult) ([]*unstructured.Unstructured, error) {
// Normalize live and target resources (cleaning or aligning them)
func normalizeTargetResources(cr *comparisonResult) ([]*unstructured.Unstructured, error) {
// normalize live and target resources
normalized, err := diff.Normalize(cr.reconciliationResult.Live, cr.reconciliationResult.Target, cr.diffConfig)
if err != nil {
return nil, err
}
patchedTargets := []*unstructured.Unstructured{}
for idx, live := range cr.reconciliationResult.Live {
normalizedTarget := normalized.Targets[idx]
if normalizedTarget == nil {
patchedTargets = append(patchedTargets, nil)
continue
}
gvk := normalizedTarget.GroupVersionKind()
originalTarget := cr.reconciliationResult.Target[idx]
if live == nil {
// No live resource, just use target
patchedTargets = append(patchedTargets, originalTarget)
continue
}
var (
lookupPatchMeta strategicpatch.LookupPatchMeta
versionedObject any
)
// Load patch meta struct or OpenAPI schema for CRDs
if versionedObject, err = scheme.Scheme.New(gvk); err == nil {
if lookupPatchMeta, err = strategicpatch.NewPatchMetaFromStruct(versionedObject); err != nil {
var lookupPatchMeta *strategicpatch.PatchMetaFromStruct
versionedObject, err := scheme.Scheme.New(normalizedTarget.GroupVersionKind())
if err == nil {
meta, err := strategicpatch.NewPatchMetaFromStruct(versionedObject)
if err != nil {
return nil, err
}
} else if crdSchema := openAPISchema.LookupResource(gvk); crdSchema != nil {
lookupPatchMeta = strategicpatch.NewPatchMetaFromOpenAPI(crdSchema)
lookupPatchMeta = &meta
}
// Calculate live patch
livePatch, err := getMergePatch(normalized.Lives[idx], live, lookupPatchMeta)
if err != nil {
return nil, err
}
// Apply the patch to the normalized target
// This ensures ignored fields in live are restored into the target before syncing
normalizedTarget, err = applyMergePatch(normalizedTarget, livePatch, versionedObject, lookupPatchMeta)
normalizedTarget, err = applyMergePatch(normalizedTarget, livePatch, versionedObject)
if err != nil {
return nil, err
}
patchedTargets = append(patchedTargets, normalizedTarget)
}
return patchedTargets, nil
}
// getMergePatch calculates and returns the patch between the original and the
// modified unstructures.
func getMergePatch(original, modified *unstructured.Unstructured, lookupPatchMeta strategicpatch.LookupPatchMeta) ([]byte, error) {
func getMergePatch(original, modified *unstructured.Unstructured, lookupPatchMeta *strategicpatch.PatchMetaFromStruct) ([]byte, error) {
originalJSON, err := original.MarshalJSON()
if err != nil {
return nil, err
@@ -509,35 +496,18 @@ func getMergePatch(original, modified *unstructured.Unstructured, lookupPatchMet
return jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
}
// applyMergePatch will apply the given patch in the obj and return the patched unstructure.
func applyMergePatch(obj *unstructured.Unstructured, patch []byte, versionedObject any, meta strategicpatch.LookupPatchMeta) (*unstructured.Unstructured, error) {
// applyMergePatch will apply the given patch in the obj and return the patched
// unstructure.
func applyMergePatch(obj *unstructured.Unstructured, patch []byte, versionedObject any) (*unstructured.Unstructured, error) {
originalJSON, err := obj.MarshalJSON()
if err != nil {
return nil, err
}
var patchedJSON []byte
switch {
case versionedObject != nil:
patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, patch, versionedObject)
case meta != nil:
var originalMap, patchMap map[string]any
if err := json.Unmarshal(originalJSON, &originalMap); err != nil {
return nil, err
}
if err := json.Unmarshal(patch, &patchMap); err != nil {
return nil, err
}
patchedMap, err := strategicpatch.StrategicMergeMapPatchUsingLookupPatchMeta(originalMap, patchMap, meta)
if err != nil {
return nil, err
}
patchedJSON, err = json.Marshal(patchedMap)
if err != nil {
return nil, err
}
default:
if versionedObject == nil {
patchedJSON, err = jsonpatch.MergePatch(originalJSON, patch)
} else {
patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, patch, versionedObject)
}
if err != nil {
return nil, err

225
controller/sync_test.go
View File

@@ -1,17 +1,9 @@
package controller
import (
"fmt"
"os"
"strconv"
"testing"
openapi_v2 "github.com/google/gnostic-models/openapiv2"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/kubectl/pkg/util/openapi"
"sigs.k8s.io/yaml"
"github.com/argoproj/gitops-engine/pkg/sync"
synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -31,29 +23,6 @@ import (
"github.com/argoproj/argo-cd/v3/util/argo/normalizers"
)
type fakeDiscovery struct {
schema *openapi_v2.Document
}
func (f *fakeDiscovery) OpenAPISchema() (*openapi_v2.Document, error) {
return f.schema, nil
}
func loadCRDSchema(t *testing.T, path string) *openapi_v2.Document {
t.Helper()
data, err := os.ReadFile(path)
require.NoError(t, err)
jsonData, err := yaml.YAMLToJSON(data)
require.NoError(t, err)
doc, err := openapi_v2.ParseDocument(jsonData)
require.NoError(t, err)
return doc
}
func TestPersistRevisionHistory(t *testing.T) {
app := newFakeApp()
app.Status.OperationState = nil
@@ -416,7 +385,7 @@ func TestNormalizeTargetResources(t *testing.T) {
f := setup(t, ignores)
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -429,7 +398,7 @@ func TestNormalizeTargetResources(t *testing.T) {
f := setup(t, []v1alpha1.ResourceIgnoreDifferences{})
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -449,7 +418,7 @@ func TestNormalizeTargetResources(t *testing.T) {
unstructured.RemoveNestedField(live.Object, "metadata", "annotations", "iksm-version")
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -474,7 +443,7 @@ func TestNormalizeTargetResources(t *testing.T) {
f := setup(t, ignores)
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -489,6 +458,7 @@ func TestNormalizeTargetResources(t *testing.T) {
assert.Equal(t, int64(4), replicas)
})
t.Run("will keep new array entries not found in live state if not ignored", func(t *testing.T) {
t.Skip("limitation in the current implementation")
// given
ignores := []v1alpha1.ResourceIgnoreDifferences{
{
@@ -502,7 +472,7 @@ func TestNormalizeTargetResources(t *testing.T) {
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -539,11 +509,6 @@ func TestNormalizeTargetResourcesWithList(t *testing.T) {
}
t.Run("will properly ignore nested fields within arrays", func(t *testing.T) {
doc := loadCRDSchema(t, "testdata/schemas/httpproxy_openapi_v2.yaml")
disco := &fakeDiscovery{schema: doc}
oapiGetter := openapi.NewOpenAPIGetter(disco)
oapiResources, err := openapi.NewOpenAPIParser(oapiGetter).Parse()
require.NoError(t, err)
// given
ignores := []v1alpha1.ResourceIgnoreDifferences{
{
@@ -557,11 +522,8 @@ func TestNormalizeTargetResourcesWithList(t *testing.T) {
target := test.YamlToUnstructured(testdata.TargetHTTPProxy)
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
gvk := schema.GroupVersionKind{Group: "projectcontour.io", Version: "v1", Kind: "HTTPProxy"}
fmt.Printf("LookupResource result: %+v\n", oapiResources.LookupResource(gvk))
// when
patchedTargets, err := normalizeTargetResources(oapiResources, f.comparisonResult)
patchedTargets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -600,7 +562,7 @@ func TestNormalizeTargetResourcesWithList(t *testing.T) {
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -652,7 +614,7 @@ func TestNormalizeTargetResourcesWithList(t *testing.T) {
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
// when
targets, err := normalizeTargetResources(nil, f.comparisonResult)
targets, err := normalizeTargetResources(f.comparisonResult)
// then
require.NoError(t, err)
@@ -706,175 +668,6 @@ func TestNormalizeTargetResourcesWithList(t *testing.T) {
assert.Equal(t, "EV", env0["name"])
assert.Equal(t, "here", env0["value"])
})
t.Run("patches ignored differences in individual array elements of HTTPProxy CRD", func(t *testing.T) {
doc := loadCRDSchema(t, "testdata/schemas/httpproxy_openapi_v2.yaml")
disco := &fakeDiscovery{schema: doc}
oapiGetter := openapi.NewOpenAPIGetter(disco)
oapiResources, err := openapi.NewOpenAPIParser(oapiGetter).Parse()
require.NoError(t, err)
ignores := []v1alpha1.ResourceIgnoreDifferences{
{
Group: "projectcontour.io",
Kind: "HTTPProxy",
JQPathExpressions: []string{".spec.routes[].rateLimitPolicy.global.descriptors[].entries[]"},
},
}
f := setupHTTPProxy(t, ignores)
target := test.YamlToUnstructured(testdata.TargetHTTPProxy)
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
live := test.YamlToUnstructured(testdata.LiveHTTPProxy)
f.comparisonResult.reconciliationResult.Live = []*unstructured.Unstructured{live}
patchedTargets, err := normalizeTargetResources(oapiResources, f.comparisonResult)
require.NoError(t, err)
require.Len(t, patchedTargets, 1)
patched := patchedTargets[0]
// verify descriptors array in patched target
descriptors := dig(patched.Object, "spec", "routes", 0, "rateLimitPolicy", "global", "descriptors").([]any)
require.Len(t, descriptors, 1) // Only the descriptors with ignored entries should remain
// verify individual entries array inside the descriptor
entriesArr := dig(patched.Object, "spec", "routes", 0, "rateLimitPolicy", "global", "descriptors", 0, "entries").([]any)
require.Len(t, entriesArr, 1) // Only the ignored entry should be patched
// verify the content of the entry is preserved correctly
entry := entriesArr[0].(map[string]any)
requestHeader := entry["requestHeader"].(map[string]any)
assert.Equal(t, "sample-header", requestHeader["headerName"])
assert.Equal(t, "sample-key", requestHeader["descriptorKey"])
})
}
func TestNormalizeTargetResourcesCRDs(t *testing.T) {
type fixture struct {
comparisonResult *comparisonResult
}
setupHTTPProxy := func(t *testing.T, ignores []v1alpha1.ResourceIgnoreDifferences) *fixture {
t.Helper()
dc, err := diff.NewDiffConfigBuilder().
WithDiffSettings(ignores, nil, true, normalizers.IgnoreNormalizerOpts{}).
WithNoCache().
Build()
require.NoError(t, err)
live := test.YamlToUnstructured(testdata.SimpleAppLiveYaml)
target := test.YamlToUnstructured(testdata.SimpleAppTargetYaml)
return &fixture{
&comparisonResult{
reconciliationResult: sync.ReconciliationResult{
Live: []*unstructured.Unstructured{live},
Target: []*unstructured.Unstructured{target},
},
diffConfig: dc,
},
}
}
t.Run("sample-app", func(t *testing.T) {
doc := loadCRDSchema(t, "testdata/schemas/simple-app.yaml")
disco := &fakeDiscovery{schema: doc}
oapiGetter := openapi.NewOpenAPIGetter(disco)
oapiResources, err := openapi.NewOpenAPIParser(oapiGetter).Parse()
require.NoError(t, err)
ignores := []v1alpha1.ResourceIgnoreDifferences{
{
Group: "example.com",
Kind: "SimpleApp",
JQPathExpressions: []string{".spec.servers[1].enabled", ".spec.servers[0].port"},
},
}
f := setupHTTPProxy(t, ignores)
target := test.YamlToUnstructured(testdata.SimpleAppTargetYaml)
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
live := test.YamlToUnstructured(testdata.SimpleAppLiveYaml)
f.comparisonResult.reconciliationResult.Live = []*unstructured.Unstructured{live}
patchedTargets, err := normalizeTargetResources(oapiResources, f.comparisonResult)
require.NoError(t, err)
require.Len(t, patchedTargets, 1)
patched := patchedTargets[0]
require.NotNil(t, patched)
// 'spec.servers' array has length 2
servers := dig(patched.Object, "spec", "servers").([]any)
require.Len(t, servers, 2)
// first server's 'name' is 'server1'
name1 := dig(patched.Object, "spec", "servers", 0, "name").(string)
assert.Equal(t, "server1", name1)
assert.Equal(t, int64(8081), dig(patched.Object, "spec", "servers", 0, "port").(int64))
assert.Equal(t, int64(9090), dig(patched.Object, "spec", "servers", 1, "port").(int64))
// first server's 'enabled' should be true
enabled1 := dig(patched.Object, "spec", "servers", 0, "enabled").(bool)
assert.True(t, enabled1)
// second server's 'name' should be 'server2'
name2 := dig(patched.Object, "spec", "servers", 1, "name").(string)
assert.Equal(t, "server2", name2)
// second server's 'enabled' should be true (respected from live due to ignoreDifferences)
enabled2 := dig(patched.Object, "spec", "servers", 1, "enabled").(bool)
assert.True(t, enabled2)
})
t.Run("rollout-obj", func(t *testing.T) {
// Load Rollout CRD schema like SimpleApp
doc := loadCRDSchema(t, "testdata/schemas/rollout-schema.yaml")
disco := &fakeDiscovery{schema: doc}
oapiGetter := openapi.NewOpenAPIGetter(disco)
oapiResources, err := openapi.NewOpenAPIParser(oapiGetter).Parse()
require.NoError(t, err)
ignores := []v1alpha1.ResourceIgnoreDifferences{
{
Group: "argoproj.io",
Kind: "Rollout",
JQPathExpressions: []string{`.spec.template.spec.containers[] | select(.name == "init") | .image`},
},
}
f := setupHTTPProxy(t, ignores)
live := test.YamlToUnstructured(testdata.LiveRolloutYaml)
target := test.YamlToUnstructured(testdata.TargetRolloutYaml)
f.comparisonResult.reconciliationResult.Live = []*unstructured.Unstructured{live}
f.comparisonResult.reconciliationResult.Target = []*unstructured.Unstructured{target}
targets, err := normalizeTargetResources(oapiResources, f.comparisonResult)
require.NoError(t, err)
require.Len(t, targets, 1)
patched := targets[0]
require.NotNil(t, patched)
containers := dig(patched.Object, "spec", "template", "spec", "containers").([]any)
require.Len(t, containers, 2)
initContainer := containers[0].(map[string]any)
mainContainer := containers[1].(map[string]any)
// Assert init container image is preserved (ignoreDifferences works)
initImage := dig(initContainer, "image").(string)
assert.Equal(t, "init-container:v1", initImage)
// Assert main container fields as expected
mainName := dig(mainContainer, "name").(string)
assert.Equal(t, "main", mainName)
mainImage := dig(mainContainer, "image").(string)
assert.Equal(t, "main-container:v1", mainImage)
})
}
func TestDeriveServiceAccountMatchingNamespaces(t *testing.T) {

12
controller/testdata/data.go vendored
View File

@@ -32,16 +32,4 @@ var (
//go:embed additional-image-replicas-deployment.yaml
AdditionalImageReplicaDeploymentYaml string
//go:embed simple-app-live.yaml
SimpleAppLiveYaml string
//go:embed simple-app-target.yaml
SimpleAppTargetYaml string
//go:embed target-rollout.yaml
TargetRolloutYaml string
//go:embed live-rollout.yaml
LiveRolloutYaml string
)

25
controller/testdata/live-rollout.yaml vendored
View File

@@ -1,25 +0,0 @@
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: rollout-sample
spec:
replicas: 2
strategy:
canary:
steps:
- setWeight: 20
selector:
matchLabels:
app: rollout-sample
template:
metadata:
labels:
app: rollout-sample
spec:
containers:
- name: init
image: init-container:v1
livenessProbe:
initialDelaySeconds: 10
- name: main
image: main-container:v1

62
controller/testdata/schemas/httpproxy_openapi_v2.yaml vendored
View File

@@ -1,62 +0,0 @@
swagger: "2.0"
info:
title: HTTPProxy
version: "v1"
paths: {}
definitions:
io.projectcontour.v1.HTTPProxy:
type: object
x-kubernetes-group-version-kind:
- group: projectcontour.io
version: v1
kind: HTTPProxy
properties:
spec:
type: object
properties:
routes:
type: array
items:
type: object
properties:
rateLimitPolicy:
type: object
properties:
global:
type: object
properties:
descriptors:
type: array
x-kubernetes-list-map-keys:
- entries
items:
type: object
properties:
entries:
type: array
x-kubernetes-list-map-keys:
- headerName
items:
type: object
properties:
requestHeader:
type: object
properties:
descriptorKey:
type: string
headerName:
type: string
requestHeaderValueMatch:
type: object
properties:
headers:
type: array
items:
type: object
properties:
name:
type: string
contains:
type: string
value:
type: string

67
controller/testdata/schemas/rollout-schema.yaml vendored
View File

@@ -1,67 +0,0 @@
swagger: "2.0"
info:
title: Rollout
version: "v1alpha1"
paths: {}
definitions:
argoproj.io.v1alpha1.Rollout:
type: object
x-kubernetes-group-version-kind:
- group: argoproj.io
version: v1alpha1
kind: Rollout
properties:
spec:
type: object
properties:
replicas:
type: integer
strategy:
type: object
properties:
canary:
type: object
properties:
steps:
type: array
items:
type: object
properties:
setWeight:
type: integer
selector:
type: object
properties:
matchLabels:
type: object
additionalProperties:
type: string
template:
type: object
properties:
metadata:
type: object
properties:
labels:
type: object
additionalProperties:
type: string
spec:
type: object
properties:
containers:
type: array
x-kubernetes-list-map-keys:
- name
items:
type: object
properties:
name:
type: string
image:
type: string
livenessProbe:
type: object
properties:
initialDelaySeconds:
type: integer

29
controller/testdata/schemas/simple-app.yaml vendored
View File

@@ -1,29 +0,0 @@
swagger: "2.0"
info:
title: SimpleApp
version: "v1"
paths: {}
definitions:
example.com.v1.SimpleApp:
type: object
x-kubernetes-group-version-kind:
- group: example.com
version: v1
kind: SimpleApp
properties:
spec:
type: object
properties:
servers:
type: array
x-kubernetes-list-map-keys:
- name
items:
type: object
properties:
name:
type: string
port:
type: integer
enabled:
type: boolean

12
controller/testdata/simple-app-live.yaml vendored
View File

@@ -1,12 +0,0 @@
apiVersion: example.com/v1
kind: SimpleApp
metadata:
name: simpleapp-sample
spec:
servers:
- name: server1
port: 8081 # port changed in live from 8080
enabled: true
- name: server2
port: 9090
enabled: true # enabled changed in live from false

12
controller/testdata/simple-app-target.yaml vendored
View File

@@ -1,12 +0,0 @@
apiVersion: example.com/v1
kind: SimpleApp
metadata:
name: simpleapp-sample
spec:
servers:
- name: server1
port: 8080
enabled: true
- name: server2
port: 9090
enabled: false

25
controller/testdata/target-rollout.yaml vendored
View File

@@ -1,25 +0,0 @@
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: rollout-sample
spec:
replicas: 2
strategy:
canary:
steps:
- setWeight: 20
selector:
matchLabels:
app: rollout-sample
template:
metadata:
labels:
app: rollout-sample
spec:
containers:
- name: init
image: init-container:v1
livenessProbe:
initialDelaySeconds: 15
- name: main
image: main-container:v1

2
go.mod
View File

@@ -47,7 +47,7 @@ require (
github.com/golang-jwt/jwt/v5 v5.3.0
github.com/golang/protobuf v1.5.4
github.com/google/btree v1.1.3
github.com/google/gnostic-models v0.7.0
github.com/google/gnostic-models v0.7.0 // indirect
github.com/google/go-cmp v0.7.0
github.com/google/go-github/v69 v69.2.0
github.com/google/go-jsonnet v0.21.0