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.

Sign up for GitHub

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

Jump to bottom

Create controller CA ConfigMap in the controller deployement Namespace #878

Merged
merged 1 commit into from
Jun 30, 2020
Merged

Create controller CA ConfigMap in the controller deployement Namespace #878

merged 1 commit into from
Jun 30, 2020

Conversation

jianjuns
Copy link
Contributor

@jianjuns jianjuns commented Jun 26, 2020 •
edited
Loading

Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: #876

@jianjuns jianjuns requested review from antoninbas and tnqn June 26, 2020 19:48
@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-whole-conformance: to trigger all conformance tests on linux.
  • /skip-whole-conformance: to skip all conformance tests on linux.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests (except whole conformance).
  • /skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

antoninbas
antoninbas reviewed Jun 26, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

Suggestion for commit message:
s/No more use the fixed "kube-system" Namespace for the CA ConfigMap./Stop using the fixed "kube-system" Namespace for the CA ConfigMap./

pkg/apiserver/certificate/cacert_controller.go Outdated Show resolved Hide resolved
pkg/apiserver/certificate/cacert_controller.go Outdated Show resolved Hide resolved
test/e2e/security_test.go Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/securing-control-plane.md Outdated Show resolved Hide resolved
pkg/apiserver/certificate/certificate.go Outdated Show resolved Hide resolved
@jianjuns jianjuns force-pushed the cert-ns branch 2 times, most recently from 7f79d96 to 2ba4d41 Compare June 27, 2020 00:41
abhiraut
abhiraut reviewed Jun 27, 2020
View reviewed changes
cmd/antrea-agent/main.go Outdated Show resolved Hide resolved
docs/securing-control-plane.md
ConfigMap named `antrea-ca` in the `kube-system` Namespace and inject it into
the APIServices resources created by Antrea in order to allow its clients (i.e.
antrea-agent, kube-apiserver) to perform authentication.
ConfigMap named `antrea-ca` in the Antrea deployment Namespace and inject it
Copy link
Contributor

Choose a reason for hiding this comment

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

deployment -> Deployment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Antonin suggested to cover both controller and agent, so I used Antrea deployment to refer to the whole deployment of Antrea components.

pkg/apiserver/certificate/certificate.go Outdated Show resolved Hide resolved
@jianjuns jianjuns force-pushed the cert-ns branch 2 times, most recently from 2552ac8 to 4c3df0e Compare June 28, 2020 17:30
tnqn
tnqn reviewed Jun 29, 2020
View reviewed changes
cmd/antrea-controller/config.go
// ca.crt: <CA certificate>
// tls.crt: <TLS certificate>
// tls.key: <TLS private key>
// Defaults to true.
// And the Secret must be mounted to directory "/var/run/antrea/antrea-controller-tls" of the
Copy link
Member

Choose a reason for hiding this comment

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

Insert it before Defaults to true.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

docs/configuration.md
# ca.crt: <CA certificate>
# tls.crt: <TLS certificate>
# tls.key: <TLS private key>
#selfSignedCert: true
# And the Secret must be mounted to directory "/var/run/antrea/antrea-controller-tls" of the
Copy link
Member

Choose a reason for hiding this comment

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

Insert it before selfSignedCert: true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@jianjuns
Create controller CA ConfigMap in the controller deployement Namespace
a135a6e 
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
@tnqn
Copy link
Member

tnqn commented Jun 29, 2020

Not sure if secret is the only one bound with kube-system namespace.
To ensure antrea works in any namespace, perhaps we could run all tests by applying antrea in a randomly created namespace? but no need to be in this PR of course.

tnqn
tnqn approved these changes Jun 29, 2020
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

abhiraut
abhiraut approved these changes Jun 29, 2020
View reviewed changes
Copy link
Contributor

@abhiraut abhiraut left a comment

Choose a reason for hiding this comment

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

lgtm

@jianjuns
Copy link
Contributor Author

/test-all

@jianjuns jianjuns merged commit a2e9594 into antrea-io:master Jun 30, 2020
@antoninbas antoninbas mentioned this pull request Jul 8, 2020
Closed
@jianjuns jianjuns deleted the cert-ns branch August 25, 2020 21:54
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
@jianjuns @GraysonWu
Create controller CA ConfigMap in the controller deployement Namespace (
88d194a 
antrea-io#878)

Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas left review comments

@tnqn tnqn tnqn approved these changes

@abhiraut abhiraut abhiraut approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ensure Antrea works in any namespace (remove any kube-system hard-coding)?
6 participants
@jianjuns @antrea-bot @tnqn @abhiraut @antoninbas @vmwclabot