Skip to content

Commit

Permalink
clear last applied configuration (#385)
Browse files Browse the repository at this point in the history 
* clear last applied configuration and disable save configuration for unstructured objects
---------

Co-authored-by: Matous Jobanek <[email protected]>
  • Loading branch information
@mfrancisc @MatousJobanek
mfrancisc and MatousJobanek authored Apr 17, 2024
1 parent 32311a6 commit d3c1484
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 7 deletions.
24 changes: 17 additions & 7 deletions pkg/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (c ApplyClient) applyObject(ctx context.Context, obj client.Object, options
if config.saveConfiguration {
// set current object as annotation
annotations := obj.GetAnnotations()
newConfiguration = getNewConfiguration(obj)
newConfiguration = GetNewConfiguration(obj)
if annotations == nil {
annotations = map[string]string{}
}
Expand All @@ -124,6 +124,7 @@ func (c ApplyClient) applyObject(ctx context.Context, obj client.Object, options
namespacedName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}
if err := c.Client.Get(ctx, namespacedName, existing); err != nil {
if apierrors.IsNotFound(err) {
obj.SetResourceVersion("") // reset resource version when creating to avoid error: resourceVersion should not be set on objects to be created
return true, c.createObj(ctx, obj, config.owner)
}
return false, errors.Wrapf(err, "unable to get the resource '%v'", existing)
Expand All @@ -133,7 +134,8 @@ func (c ApplyClient) applyObject(ctx context.Context, obj client.Object, options
if !config.forceUpdate {
existingAnnotations := existing.GetAnnotations()
if existingAnnotations != nil {
if newConfiguration == existingAnnotations[LastAppliedConfigurationAnnotationKey] {
lastApplied, lastAppliedFound := existingAnnotations[LastAppliedConfigurationAnnotationKey]
if lastAppliedFound && newConfiguration != "" && newConfiguration == lastApplied {
return false, nil
}
}
Expand Down Expand Up @@ -205,15 +207,23 @@ func clusterIP(obj runtime.Object) (string, bool, error) {
}
}

func getNewConfiguration(newResource runtime.Object) string {
newJSON, err := marshalObjectContent(newResource)
func GetNewConfiguration(newResource client.Object) string {
// reset the previous config to avoid recursive embedding of the object
copyResource := removeAnnotation(newResource, LastAppliedConfigurationAnnotationKey)
newJSON, err := marshalObjectContent(copyResource)
if err != nil {
log.Error(err, "unable to marshal the object", "object", newResource)
return fmt.Sprintf("%v", newResource)
log.Error(err, "unable to marshal the object", "object", copyResource)
return fmt.Sprintf("%v", copyResource)
}
return string(newJSON)
}

func removeAnnotation(newResource client.Object, annotationKey string) client.Object {
copyResource := newResource.DeepCopyObject().(client.Object)
delete(copyResource.GetAnnotations(), annotationKey)
return copyResource
}

func marshalObjectContent(newResource runtime.Object) ([]byte, error) {
if newRes, ok := newResource.(runtime.Unstructured); ok {
return json.Marshal(newRes.UnstructuredContent())
Expand Down Expand Up @@ -278,7 +288,7 @@ func ApplyUnstructuredObjectsWithNewLabels(ctx context.Context, cl client.Client
for _, unstructuredObj := range unstructuredObjects {
log.Info("applying object", "object_namespace", unstructuredObj.GetNamespace(), "object_name", unstructuredObj.GetObjectKind().GroupVersionKind().Kind+"/"+unstructuredObj.GetName())
MergeLabels(unstructuredObj, newLabels)
_, err := applyClient.ApplyObject(ctx, unstructuredObj)
_, err := applyClient.ApplyObject(ctx, unstructuredObj, SaveConfiguration(false))
if err != nil {
return err
}
Expand Down
100 changes: 100 additions & 0 deletions pkg/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,106 @@ func TestApplySingle(t *testing.T) {
assert.Equal(t, "second-value", configMap.Data["first-param"])
})
})

t.Run("updates of ServiceAccount", func(t *testing.T) {

t.Run("last-applied-configuration is used and SA secret ref is not updated", func(t *testing.T) {
// given
// there's an existing SA with secret refs
existingSA := newSA()
secretRefs := []corev1.ObjectReference{
{
Name: "secret",
Namespace: existingSA.Namespace,
},
}
existingSA.Secrets = secretRefs
existingLastAppliedAnnotation := map[string]string{
client.LastAppliedConfigurationAnnotationKey: client.GetNewConfiguration(existingSA),
}
existingSA.SetAnnotations(existingLastAppliedAnnotation) // let's set the last applied annotation
cl, cli := newClient(t)
_, err := cl.ApplyRuntimeObject(context.TODO(), existingSA.DeepCopyObject())
require.NoError(t, err)

// when
// we update with existing annotations
newSA := existingSA.DeepCopy()
newSA.SetAnnotations(existingLastAppliedAnnotation) // let's set the last applied annotation
_, err = cl.ApplyRuntimeObject(context.TODO(), newSA) // then

// then
require.NoError(t, err)
var actualSa corev1.ServiceAccount
err = cli.Get(context.TODO(), types.NamespacedName{Name: "appstudio-user-sa", Namespace: "john-dev"}, &actualSa) // assert sa was created
require.NoError(t, err)
assert.Equal(t, secretRefs, actualSa.Secrets) // secret refs are still there
assert.Equal(t, existingLastAppliedAnnotation[client.LastAppliedConfigurationAnnotationKey], actualSa.Annotations[client.LastAppliedConfigurationAnnotationKey]) // the last apply configuration should match the previous object
})
})

t.Run("should update object when save configuration is disabled and another annotation is present", func(t *testing.T) {
// given
cl, cli := newClient(t)
existingRules := []rbac.PolicyRule{
{
APIGroups: []string{"toolchain.dev.openshift.com"},
Resources: []string{"bannedusers", "masteruserrecords"},
Verbs: []string{"*"},
},
}
existingRole := rbac.Role{
ObjectMeta: metav1.ObjectMeta{
Name: "toolchaincluster-host",
Namespace: HostOperatorNs,
Annotations: map[string]string{"kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"rbac.authorization.k8s.io/v1\",\"kind\":\"Role\",\"metadata\":{\"annotations\":{},\"name\":\"toolchaincluster-host\",\"namespace\":\"toolchain-host-operator\"},\"rules\":[{\"apiGroups\":[\"toolchain.dev.openshift.com\"],\"resources\":[\"bannedusers\",\"masteruserrecords\"],\"verbs\":[\"*\"]}]}"},
},
Rules: existingRules,
}

// when
_, err := cl.ApplyRuntimeObject(context.TODO(), &existingRole, client.SaveConfiguration(false))
require.NoError(t, err)

// then
// the object should match the existing role
foundRole := &rbac.Role{}
err = cli.Get(context.TODO(), types.NamespacedName{
Name: "toolchaincluster-host",
Namespace: HostOperatorNs,
}, foundRole)
require.NoError(t, err)
require.Equal(t, existingRules, foundRole.Rules)

// when
// we updated the role
newRules := []rbac.PolicyRule{
{
APIGroups: []string{"toolchain.dev.openshift.com"},
Resources: []string{"*"},
Verbs: []string{"*"},
},
}
newRole := rbac.Role{
ObjectMeta: metav1.ObjectMeta{
Name: "toolchaincluster-host",
Namespace: HostOperatorNs,
},
Rules: newRules,
}
_, err = cl.ApplyRuntimeObject(context.TODO(), &newRole, client.SaveConfiguration(false))
require.NoError(t, err)

// then
// the new rules should be there
foundRole = &rbac.Role{}
err = cl.Get(context.TODO(), types.NamespacedName{
Name: "toolchaincluster-host",
Namespace: HostOperatorNs,
}, foundRole)
require.NoError(t, err)
require.Equal(t, newRules, foundRole.Rules)
})
}

func toUnstructured(obj runtime.Object) (*unstructured.Unstructured, error) {
Expand Down

0 comments on commit d3c1484

Please sign in to comment.