pilot: do not skip creating istio-ca-root-cert in kube-system namespace #49261
Conversation
Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
istio-testing
added
the
size/S
| @@ -75,10 +79,12 @@ func NewNamespaceController(kubeClient kube.Client, caBundleWatcher *keycertbund | |||
| ObjectFilter: c.GetFilter(), | |||
| }) | |||
| c.namespaces = kclient.New[*v1.Namespace](kubeClient) | |||
| // kube-system is not skipped to enable deploying ztunnel in that namespace | |||
There was a problem hiding this comment.
I think in InsertDataToConfigMap we will want to add errors.IsUnauthorized() to the error handling and log at info level (something like "Skipping kube-system due to lack of permission") without retries?
Otherwise we are going to retry 5 times ( and could retrigger if the namespace changes) with a concerning-looking error message?
There was a problem hiding this comment.
Let me spin up a cluster that denies kube-system so I can make this more concrete maybe (and make sure IsUnauthorized is correct)
There was a problem hiding this comment.
actually its IsForbidden we want I think (at least on GKE...):
if errors.IsForbidden(err) {
log.Infof("skip writing ConfigMap %v/%v as we do not have permissions to do so", meta.Namespace, meta.Name)
return nil
}
There was a problem hiding this comment.
Thanks for suggestion. I simulated the forbidden issue on kind with this patch and it works as expected.
Before:
2024-02-08T20:23:51.593609Z error controllers error handling kube-system, retrying (retry count: 1): error when creating configmap istio-ca-root-cert: configmaps is forbidden: User "system:serviceaccount:istio-system:istiod" cannot create resource "configmaps" in API group "" in the namespace "kube-system" controller=namespace controller
...
After:
2024-02-08T20:20:22.403813Z info skip writing ConfigMap kube-system/istio-ca-root-cert as we do not have permissions to do so
…orbidden Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
istio-testing
added
size/M
|
thanks! |
…ce (istio#49261) * pilot: do not skip creating istio-ca-root-cert in kube-system namespace Signed-off-by: Jacek Ewertowski <jewertow@redhat.com> * Add a release note Signed-off-by: Jacek Ewertowski <jewertow@redhat.com> * Add a comment Signed-off-by: Jacek Ewertowski <jewertow@redhat.com> * Skip creating istio-ca-root-cert in kube-system if the namespace is forbidden Signed-off-by: Jacek Ewertowski <jewertow@redhat.com> --------- Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
|
/cherry-pick release-1.21 |
|
@dhawton: new pull request created: #49869 In response to this:
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. |
Please provide a description of this PR:
This is a simplified approach to #49129, which we discussed on WG ambient meeting. We agreed that istiod will try to create
istio-ca-root-certin thekube-systemnamespace. I did not add any special handling of an error that might be returned for kube-system, because I don't know how different distros restrict it. I guess it would be a permission error, but then it couldn't be distinguished from missing permissions on any other cluster.I created another PR to not lose the context of the previous approach.