Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RBAC auth and CA controller to Theia manager #113

Merged
merged 2 commits into from
Oct 7, 2022

Conversation

wsquan171
Copy link
Contributor

This PR aims to enable Theia manager API server access in a secured and authenticated fashion, which brings in the following changes:

  • RBAC delegation to kube-apiserver is added. API authorization of Theia manager API endpoints are now administered based on API groups and resources
  • ServiceAccount / ClusterRole / ClusterRoleBinding are added to allow Theia CLI to access API server. token from theia-cli-account-token should be set as Authorization: Bearer in HTTP header when making requests.
  • CA cert controller is added to expose CA cert used for API server via configmap theia-ca. When adding the ca.crt in the configmap to trust chain, HTTPs requests can be made in "secure" fashion.

To test these out, after Theia manager from this branch is deployed and running:

  • TOKEN=$(kubectl get secret theia-cli-account-token -n flow-visibility -o json | jq -Mr '.data.token' | base64 -d)
  • kubectl get cm theia-ca -n flow-visibility -o jsonpath='{.data.ca\.crt}' > ca-cert.pem
  • From cpVM curl -vvv --cacert ca-cert.pem https://theia-manager.flow-visibility.svc:11347/apis/intelligence.theia.antrea.io/v1alpha1/networkpolicyrecommendations/pr-test --header "Authorization: Bearer $TOKEN" --resolve theia-manager.flow-visibility.svc:11347:<ClusterIPServiceAddr>
  • From out-of-cluster via port-forwarding curl -vvv --cacert cert.pem https://localhost:<LocalForwardPort>/apis/intelligence.theia.antrea.io/v1alpha1/networkpolicyrecommendations/pr-test --header "Authorization: Bearer $TOKEN"

Copy link
Contributor

@yuntanghsu yuntanghsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Just some nits.

pkg/apiserver/apiserver.go Outdated Show resolved Hide resolved
Comment on lines 169 to 170
klog.Infof("Starting CACertController")
defer klog.Infof("Shutting down CACertController")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use klog.InfoS instead of klog.Infof? Same suggestion for the other logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. fixed other occurrences as well.

@wsquan171
Copy link
Contributor Author

/theia-test-e2e

This change adds k8s auth delegation to theia manager, and adds template
of cli service account / cluster role to allow access for specified API
groups and resources. A SA toekn secret is also added that can be used
for CLI to auth with Theia manager.

Signed-off-by: Shawn Wang <wshaoquan@vmware.com>
This change adds certificate controller to Theia manager. The public key
of API server TLS in case of self-signed, or CA cert in case of user
provided TLS, will be exposed to clients via configmap "theia-ca" in
flow-visibility namespace. This will allow cURL or client requests to be
made in "secure" fashion if the ca cert is added to trust chain.

The configmap will be updated when user provided TLS bundle is changed,
or the self-signed cert is rotated upon expiration.

Signed-off-by: Shawn Wang <wshaoquan@vmware.com>
@wsquan171
Copy link
Contributor Author

/theia-test-e2e

Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wsquan171 wsquan171 merged commit 470e08e into antrea-io:main Oct 7, 2022
@wsquan171 wsquan171 deleted the mgr-auth branch October 8, 2022 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants