Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Add namespace read support to the connect-inject clusterrole so we can read namespaces #942

Merged
merged 2 commits into from
May 5, 2021

Conversation

kschoche
Copy link
Contributor

@kschoche kschoche commented May 5, 2021

Add namespace read support to the clusterrole so we can read namespaces in the endpoints controller and webhook.

Changes proposed in this PR:

  • Add namespaces get to the connect-inject clusterrole. The webhook handler and the endpoints controller both read namespaces now to fetch metadata related to tproxy labels.

How I've tested this PR:
manually tested alongside hashicorp/consul-k8s#510 and verified that it resolves the following error:

Error from server: error when creating "svc.yaml": admission webhook "kyle-consul-consul-connect-injector.consul.hashicorp.com" denied the request: error getting namespace metadata for container: namespaces "default" is forbidden: User "system:serviceaccount:default:kyle-consul-consul-connect-injector-webhook-svc-account" cannot get resource "namespaces" in API group "" in the namespace "default"

How I expect reviewers to test this PR:
code review

Checklist:

  • Bats tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@kschoche kschoche added the area/connect Related to Connect, e.g. injection label May 5, 2021
@kschoche kschoche requested a review from a team May 5, 2021 18:01
@kschoche kschoche self-assigned this May 5, 2021
@kschoche kschoche requested review from ndhanushkodi and ishustava and removed request for a team May 5, 2021 18:01
@kschoche kschoche merged commit af4bd0c into master May 5, 2021
@kschoche kschoche deleted the tproxy_add_namespace_toggle_support branch May 5, 2021 20:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/connect Related to Connect, e.g. injection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants