kubernetes: Running conformance test on 1.29.0-alpha.3 cluster with sonobuoy fails

it looks like the e2e change made in https://github.com/kubernetes/kubernetes/pull/121283 was not consciously made part of conformance, it was just trying to functionally test the bugfix being made.

For 1.29, I think we should revert the change to test/e2e/apimachinery/aggregator.go and make an integration test that validates the fix instead. We can revisit whether content of / is part of conformance or not for 1.30

@neolit123 +1 for what @BenTheElder is saying, that we should expect that the user running the tests has required permissions. These tests are not built for being run unprivileged.

We probably need another issue to track 1.30 follow-up?

@BenTheElder If no one has done it, I could create another issue and follow up with the items that have been discussed here:

1. Update outdated privilege requirement in https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md.
2. Make commonly used conformance tools like sonobuoy aware of the required priviledge to avoid similar situations in the future.
3. Add permission of GET “/” to authorized users via the system:discovery role for root discovery.

Would have expected it here:

{
	// a role which provides just enough power to determine if the server is
	// ready and discover API versions for negotiation
	ObjectMeta: metav1.ObjectMeta{Name: "system:discovery"},
	Rules: []rbacv1.PolicyRule{
		rbacv1helpers.NewRule("get").URLs(
			"/livez", "/readyz", "/healthz",
			"/version", "/version/",
			"/openapi", "/openapi/*",
			"/api", "/api/*",
			"/apis", "/apis/*",
		).RuleOrDie(),
	},
},

The PR in question explicitly fixes the result returned by /. If that is part of conformance (IMO it should), every conformance test needs access it obviously. Whether we require admin, or we add it manually, either would work.

In other words, just add the / permission. While thinking about it, it is surprising that the user in question does not have this access already. Don’t we allow that for every authorized user?

looks like we don’t.

$ kubectl get clusterrole cluster-admin -o yaml

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  annotations:
    rbac.authorization.kubernetes.io/autoupdate: "true"
  creationTimestamp: "2023-11-24T18:34:38Z"
  labels:
    kubernetes.io/bootstrapping: rbac-defaults
  name: cluster-admin
  resourceVersion: "1206"
  uid: 13836a28-6c0d-4008-b566-bb528011e6e0
rules:
- apiGroups:
  - '*'
  resources:
  - '*'
  verbs:
  - '*'
- nonResourceURLs:
  - /api # <------------- modified from the default of '*'
  verbs:
  - '*'

$ kubectl get --raw /
Error from server (Forbidden): forbidden: User "kubernetes-admin" cannot get path "/"

$ kubectl get --raw /api
{"kind":"APIVersions","versions":["v1"],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0","serverAddress":"10.0.2.15:6443"}]}

In contrast, I hope that lots of privileged operations are tested by conformance today. If we don’t test things like admission webhooks for example, we have much bigger problems. And admission webhooks are basically equivalent to cluster-admin.

before this ticket, i was under the impression that the conformance suite already requires admin and that all testers (like Sonobuoy) use admin level credentials.

it is non-privileged (e.g., does not require root on nodes, access to raw network interfaces, or cluster admin permissions)

that is outdated, we use hostNetwork pods one some of the tests IIRC

looks like we may have to change that sentence completely.

IMHO this is a bug in running the tests via a pod, which is not how we develop the tests (they’re run outside the cluster, by a cluster admin account typically).

I would say 1) is the correct answer. xref: #121986 (comment)

@BenTheElder thanks for your comment. I guess 1) means non of the existing sonobuoy versions can pass K8s conformance test against a cluster > 1.29.0-alpha3, as the RBAC rule is hardcoded. I will file an issue to sonobuoy first.