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

Respect allow/deny lists even when namespaces aren't enabled #297

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

adilyse
Copy link
Contributor

@adilyse adilyse commented Jul 20, 2020

Fixes #296

Changes proposed in this PR:

  • Respect allow/deny list flags when namespaces are disabled.

How I've tested this PR:
Added tests to cover this scenario. Since the only functionality change is to the shouldInject method, this is sufficient.

How I expect reviewers to test this PR:
Confirm that the TestShouldInject test in connect-inject/handler_test.go works as expected and covers all the desired scenarios.

Checklist:

  • [ x ] Tests added
  • [ x ] CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@adilyse adilyse force-pushed the inject-namespace-lists branch from 7dc0d15 to ba63098 Compare July 21, 2020 16:52
@adilyse adilyse added the area/connect Related to Connect service mesh, e.g. injection label Jul 21, 2020
@adilyse adilyse marked this pull request as ready for review July 21, 2020 17:01
@adilyse adilyse requested review from a team, kschoche and thisisnotashwin and removed request for a team July 21, 2020 17:01
Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

This is most excellent

@adilyse adilyse merged commit 34896cb into master Jul 21, 2020
@adilyse adilyse deleted the inject-namespace-lists branch July 21, 2020 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connect Related to Connect service mesh, e.g. injection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-allow-k8s-namespace and -deny-k8s-namespace flags ignored
3 participants