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

Allow explicit network policy enablement #381

Merged
merged 3 commits into from
Sep 16, 2020
Merged

Allow explicit network policy enablement #381

merged 3 commits into from
Sep 16, 2020

Conversation

corest
Copy link
Contributor

@corest corest commented Sep 1, 2020

For now, the only way to create a network policy - is to enable openshift.
I don't need all the resources enabled by openshift configuration.
But I still need network policy.
This PR allows configuring network policy creation without enabling openshift

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 1, 2020

CLA assistant check
All committers have signed the CLA.

@jasonodonnell
Copy link
Contributor

Hi @corest, thanks for the contribution.

This sounds like a good idea but is a little confusing since you may want to install in OpenShift but have the policy disabled. I think we can just make this configurable using networkPolicy.enabled. Thoughts?

Additionally you'll need to add unit tests to make sure this configurable works as expected. You can find the bats unit tests in test/unit: https://github.com/hashicorp/vault-helm/blob/master/test/unit/server-network-policy.bats

@corest
Copy link
Contributor Author

corest commented Sep 2, 2020

This sounds like a good idea but is a little confusing since you may want to install in OpenShift but have the policy disabled. I think we can just make this configurable using networkPolicy.enabled. Thoughts?

I wanted to do that first but didn't want to break compatibility for someone who is using openshift configuration already and relies on it for network policy deployment. But I agree that the network policy configuration should not be something we deploy or not based on specific cloud env. I'll change it with networkPolicy.enabled only

@corest
Copy link
Contributor Author

corest commented Sep 3, 2020

@jasonodonnell PTAL

@corest
Copy link
Contributor Author

corest commented Sep 14, 2020

Can we merge this @jasonodonnell ?

Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

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

LGTM. @tvoran can you also take a look at this? It's changing network policy default behavior but since NP isn't OpenShift specific, I'm recommending we break it out as such.

Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

@jasonodonnell Seems ok to me, and we'll probably want to add a note about it to our OpenShift docs once it's released.

@tvoran tvoran merged commit 66ea34c into hashicorp:master Sep 16, 2020
@corest corest deleted the configurable_network_policy branch September 16, 2020 06:42
@jasonodonnell jasonodonnell mentioned this pull request Oct 20, 2020
ryanmt added a commit to ryanmt/vault-helm that referenced this pull request Mar 9, 2021
Similar to the conversion around PR hashicorp#381, network policies are useful
for the injector independent of openshift.  This allows support for
  those use cases but similarly will require some configuration changes
  for openshift users.
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.

4 participants