reconcile for externally managed secret is not triggered for update event

[AshfaqMH]

Discussed in #3144

^{Originally posted by AshfaqMH January 4, 2023}
I am trying to watch an externally managed secret which has a spec field in the CR. Every CRUD on the CR triggers a reconcile to validate the fields in the CR and also validates the content of the secret.

The secret is not owned by the CR. It is externally created and managed. For every CRUD on the secret , would like check in which CR the secret name is reference and trigger reconcile on that CR for the validations to through.

Below is the details :

operator-sdk version: "v1.25.2", commit: "b63b921837de8dd6ce480033e427ecfc5e34abcc", kubernetes version: "1.25.0", go version: "go1.19.2", GOOS: "linux", GOARCH: "amd64"

This is almost the same use case which is shown in the below document,

https://book.kubebuilder.io/reference/watching-resources/externally-managed.html

I have implemented exactly same as shown in the above document. The document is based on the confimap but my use case is for secret watch.

On a CR CRUD event the reconcile triggers and work perfectly fine.

But ,

• If the secret name is referenced in the CR and not created/found in the k8s cluster , the reconcile throws error and update the CR to "ERROR" status. Once the secret is created the loop reconcile picks the secret and validates and update the CR to "READY" status.

• If CR is in READY state and I delete the secret , reconcile is triggered and throws error secret not found and update the CR to "ERROR" status

• If the secret is updated/edited , no reconcile is triggered.

Am I missing something ?

Below is the implementation

const (
	secretNameField         = ".spec.secretRef"
)

// SetupWithManager sets up the controller with the Manager.
func (r *ResourceReconciler) SetupWithManager(mgr ctrl.Manager) error {
	if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1.Resource{}, secretNameField, func(rawObj client.Object) []string {
		// Extract the SecretRef name from the Resource Spec, if one is provided
		nResource := rawObj.(*v1.Resource)
		if nResource.Spec.SecretRef == "" {
			return nil
		}
		return []string{nResource.Spec.SecretRef}
	}); err != nil {
		return err
	}
	manager := ctrl.NewControllerManagedBy(mgr).
		For(&v1.Resource{}).
		WithEventFilter(ignoreStatusUpdatePredicate()).
		Watches(
			&source.Kind{Type: &corev1.Secret{}},
			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSecret),
			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}))
	return manager.Complete(r)
}
func (r *ResourceReconciler) findObjectsForSecret(secretName client.Object) []reconcile.Request {
	nCRs := &v1.ResourceList{}
	listOps := &client.ListOptions{
		Namespace:     secretName.GetNamespace(),
		FieldSelector: fields.OneTermEqualSelector(secretNameField, secretName.GetName()),
	}
	err := r.List(context.TODO(), nCRs, listOps)
	if err != nil {
		return []reconcile.Request{}
	}
requests := make([]reconcile.Request, len(nCRs.Items))
	for i, item := range nCRs.Items {
		requests[i] = reconcile.Request{
			NamespacedName: types.NamespacedName{
				Name:      item.GetName(),
				Namespace: item.GetNamespace(),
			},
		}
	}
	return requests
}
func ignoreStatusUpdatePredicate() predicate.Predicate {
	return predicate.Funcs{
		UpdateFunc: func(e event.UpdateEvent) bool {
			// Ignore updates to CR status in which case metadata.Generation does not change.
			return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()
		},
	}
}

[AshfaqMH]

The secret update is triggering reconcile now. Issue was with the ignoreStatusUpdatePredicate() predicate which was suppressing the reconcile.
But the predicate was added for ignoring the reconcile when the status of the CR is updated.
To handle this , have enhanced the predicate function using the object type.

Below is the code snippet.

func ignoreStatusUpdatePredicate() predicate.Predicate {
	return predicate.Funcs{
		UpdateFunc: func(e event.UpdateEvent) bool {
			// Reconcile if the resource version of the secret is changed.
			if reflect.TypeOf(e.ObjectOld).String() == "*v1.Secret" {
				return e.ObjectOld.GetResourceVersion() != e.ObjectNew.GetResourceVersion()
			}
			// Ignore updates to CR status in which case metadata.Generation does not change.
			return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()
		},
	}
}

So now for any changes to secret or the CR , the reconciles are triggered.

Feel free to provide any feedback or suggestions.

[AshfaqMH]

I have a follow up requirement from the team to have a label selector on the secrets(externally managed resource) so that all the secrets in the namespace are not watched.
I found a similar requirement discussion in the below link.
kubernetes-sigs/controller-runtime#244

Anyone please suggest how can we implement it in my case.

[AshfaqMH]

Hi @camilamacedo86 , With the above code I get the error "the object has been modified; please apply your changes to the latest version and try again" when there is updated on secret followed by update on the CR.
Could you please help me .

[camilamacedo86]

See that when the object is not owned you can let the watch feature knows. Example with the old syntax: https://github.com/dev4devs-com/postgresql-operator/blob/master/pkg/controller/backup/controller.go#L60-L63

See the implementation:

err := c.Watch(&source.Kind{Type: obj}, &handler.EnqueueRequestForOwner{
		IsController: isConttroller,  // here needs to be false when the resource is not owned by the controller
		OwnerType:    owner,
	})

See the controller-runtime implementation: https://github.com/kubernetes-sigs/controller-runtime/blob/613648eda7831bea8928e9fb1e3ea06df5f3851a/pkg/handler/enqueue_owner.go#L70-L82

Please, let us know if it answer your question.

[AshfaqMH]

Thank you for your response @camilamacedo86 .
The watch functionality I have handled it in the same controller using watches as below.

	manager := ctrl.NewControllerManagedBy(mgr).
		For(&v1.Resource{}).
		WithEventFilter(ignoreStatusUpdatePredicate()).
		Watches(
			&source.Kind{Type: &corev1.Secret{}},
			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSecret),
			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}))
	return manager.Complete(r)

With this all the secrets in the cluster will be watched so i wanted to know if we can add labels and only labelled secret can be watched.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.