errors.IsUnauthorized(err) no longer works with controller-runtime 0.15.0

[bobdonat]

After calling client.Get we are checking for Unauthorized using errors.IsUnauthorized(err). This no longer works with controller-runtime 0.15.0. err is a wrapped error now which seems to be the root of the problem. errors is imported from k8s.io/apimachinery/pkg/api/errors. Is this expected behavior? I did not see any mention of this breakage in the release notes.

[bobdonat]

Will we see the same behavior with IsForbidden, IsConflict, etc.?

[bobdonat]

We have this code which handled the IsUnauthorized case with controller-runtime 0.13.2 but does not with controller-runtime 0.15.0. This code used to bypass the if block but now does not with 0.15.0 when unauthorized.

	err := s.AdminClient.Get(s.Context, vmcName, &vmc)
	if client.IgnoreNotFound(err) != nil && !apierrors.IsUnauthorized(err)) {
		s.Log.Errorf("Failed to get the VMC resources %s/%s from the admin cluster: %v", constants.VerrazzanoMultiClusterNamespace, s.ManagedClusterName, err)
		return false, err
	}

[troy0820]

/kind support

1. This was discussed in the kubernetes slack indicating that there should not be errors that were being received from the cache.
2. The error is not wrapped as apimachinery checks these https://github.com/kubernetes/kubernetes/blob/9410de78b23eb4f88de900552d90638bc67ffcd4/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go#L810
3. If it was giving errors before, it was a bug.

Moving from v0.13.2 to v0.15.0 indicated some breaking changes that the users need to be aware of. Making a big jump to two minor version can face serious impact of change around this library and others that are shared between controller-runtime and k8s.io/*

You may need to read from a different client APIReader to see if those changes are getting the errors you desire instead of the cache that the client.Client is reading from being that they should have access to the internal store where it reads from.

[bobdonat]

Had to use this local function instead of just calling apierrors.IsUnauthorized(err) to get the same behavior as 0.13.2.

	groupErr := &discovery.ErrGroupDiscoveryFailed{}
	if errors.As(err, &groupErr) {
		for _, err := range groupErr.Groups {
			if apierrors.IsUnauthorized(err) {
				return true
			}
		}
	}

Seems like a breaking change to me that is not documented.

I have seen others had to use something similar to replace apierrors.IsNotFound(err).

[troy0820]

This looks related #2354 and the fixed was merged in main #2472

[troy0820]

This landed in v0.16.2 release

[bobdonat]

Will this be backported to 0.15?

Looks related but the PR mentions fixes use of meta.IsNoMatchError() which is not what I am using

[troy0820]

I'm not sure, if I make the backport, it will still need to be released by the maintainers.

[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.