Skip to content

Commit

Permalink
Fix #1936: avoid patching if the target resource contains all expecte…
Browse files Browse the repository at this point in the history
…d fields
  • Loading branch information
nicolaferraro committed Jan 22, 2021
1 parent 0d741a7 commit 3b992da
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 12 deletions.
2 changes: 2 additions & 0 deletions pkg/trait/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ func mountRegistrySecret(name string, secret registrySecret, volumes *[]corev1.V
*volumeMounts = append(*volumeMounts, corev1.VolumeMount{
Name: "registry-secret",
MountPath: secret.mountPath,
ReadOnly: true,
})

if secret.refEnv != "" {
Expand Down Expand Up @@ -530,6 +531,7 @@ func mountRegistryConfigMap(name string, config registryConfigMap, volumes *[]co
*volumeMounts = append(*volumeMounts, corev1.VolumeMount{
Name: "registry-config",
MountPath: config.mountPath,
ReadOnly: true,
})
}

Expand Down
28 changes: 16 additions & 12 deletions pkg/trait/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package trait

import (
"github.com/pkg/errors"

"k8s.io/apimachinery/pkg/types"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -95,17 +94,22 @@ func (t *deployerTrait) Apply(e *Environment) error {
return err
}

p, err := patch.PositiveMergePatch(object, resource)
if err != nil {
return err
} else if len(p) == 0 {
// Avoid triggering a patch request for nothing
continue
}

err = env.Client.Patch(env.C, resource, client.RawPatch(types.MergePatchType, p))
if err != nil {
return errors.Wrap(err, "error during patch resource")
if !patch.SpecEqualDeepDerivative(object, resource) {
// If both objects have a "Spec" field and it contains all expected fields
// (plus optional others), then avoid patching

p, err := patch.PositiveMergePatch(object, resource)
if err != nil {
return err
} else if len(p) == 0 {
// Avoid triggering a patch request for nothing
continue
}

err = env.Client.Patch(env.C, resource, client.RawPatch(types.MergePatchType, p))
if err != nil {
return errors.Wrap(err, "error during patch resource")
}
}
}
return nil
Expand Down
5 changes: 5 additions & 0 deletions pkg/trait/trait_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ func (e *Environment) ConfigureVolumesAndMounts(vols *[]corev1.Volume, mnts *[]c
*mnts = append(*mnts, corev1.VolumeMount{
Name: refName,
MountPath: resPath,
ReadOnly: true,
})
}

Expand Down Expand Up @@ -603,6 +604,7 @@ func (e *Environment) ConfigureVolumesAndMounts(vols *[]corev1.Volume, mnts *[]c
*mnts = append(*mnts, corev1.VolumeMount{
Name: refName,
MountPath: resPath,
ReadOnly: true,
})
}

Expand Down Expand Up @@ -639,6 +641,7 @@ func (e *Environment) ConfigureVolumesAndMounts(vols *[]corev1.Volume, mnts *[]c
*mnts = append(*mnts, corev1.VolumeMount{
Name: propertiesType + "-properties",
MountPath: mountPath,
ReadOnly: true,
})
}
})
Expand All @@ -665,6 +668,7 @@ func (e *Environment) ConfigureVolumesAndMounts(vols *[]corev1.Volume, mnts *[]c
*mnts = append(*mnts, corev1.VolumeMount{
Name: refName,
MountPath: path.Join(ConfigMapsMountPath, strings.ToLower(cmName)),
ReadOnly: true,
})
}

Expand All @@ -687,6 +691,7 @@ func (e *Environment) ConfigureVolumesAndMounts(vols *[]corev1.Volume, mnts *[]c
*mnts = append(*mnts, corev1.VolumeMount{
Name: refName,
MountPath: path.Join(SecretsMountPath, strings.ToLower(secretName)),
ReadOnly: true,
})
}

Expand Down
20 changes: 20 additions & 0 deletions pkg/util/patch/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"reflect"

jsonpatch "github.com/evanphx/json-patch"
"k8s.io/apimachinery/pkg/api/equality"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/json"
Expand Down Expand Up @@ -89,3 +90,22 @@ func removeNilValues(v reflect.Value, parent reflect.Value) {
}
}
}

func SpecEqualDeepDerivative(object runtime.Object, expected runtime.Object) (res bool) {
defer func() {
if r := recover(); r != nil {
res = false
}
}()

if expected == nil {
return true
} else if object == nil {
return false
}

objectSpec := reflect.ValueOf(object).Elem().FieldByName("Spec").Interface()
expectedSpec := reflect.ValueOf(expected).Elem().FieldByName("Spec").Interface()

return equality.Semantic.DeepDerivative(expectedSpec, objectSpec)
}

0 comments on commit 3b992da

Please sign in to comment.