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

Adapt NetworkPolicys according to recent refactorings #162

Merged
merged 7 commits into from
Mar 13, 2023

Conversation

ScheererJ
Copy link
Member

@ScheererJ ScheererJ commented Mar 7, 2023

How to categorize this PR?

/area dev-productivity ops-productivity networking security
/kind enhancement

What this PR does / why we need it:
This PR adapts the NetworkPolicys according to the recent refactorings in gardener/gardener
See also gardener/gardener#7589 and https://github.com/gardener/gardener/blob/master/docs/concepts/resource-manager.md#networkpolicy-controller.

Which issue(s) this PR fixes:
Part of gardener/gardener#7352

Special notes for your reviewer:
/cc @rfranzke @oliver-goetz @timuthy

Depends on gardener/gardener@v1.66 to create the expected network policies in the extension namespace.

Release note:

Adapted extension components to support the [FullNetworkPoliciesInRuntimeCluster](https://github.com/gardener/gardener/blob/master/docs/deployment/feature_gates.md#list-of-feature-gates) feature gate introduced by `gardener/gardener` v1.66, see [here](https://github.com/gardener/gardener/blob/master/docs/concepts/resource-manager.md#networkpolicy-controller) and [#7352](https://github.com/gardener/gardener/pull/7589) for more information.

@gardener-robot gardener-robot added area/dev-productivity Developer productivity related (how to improve development) area/networking Networking related area/ops-productivity Operator productivity related (how to improve operations) area/security Security related kind/enhancement Enhancement, improvement, extension labels Mar 7, 2023
@gardener-robot
Copy link

@ScheererJ Thank you for your contribution.

@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Mar 7, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 7, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 7, 2023
@ScheererJ ScheererJ force-pushed the enhancement/network-policies branch from 0da8f3f to 58a0f6b Compare March 10, 2023 14:20
@ScheererJ ScheererJ marked this pull request as ready for review March 10, 2023 14:21
@ScheererJ ScheererJ requested review from a team as code owners March 10, 2023 14:21
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 10, 2023
Copy link
Member

@DockToFuture DockToFuture left a comment

Choose a reason for hiding this comment

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

I guess we need here to also set the metrics port on the svc.

@ScheererJ
Copy link
Member Author

I guess we need here to also set the metrics port on the svc.

We could also explicitly define the metrics port in the service. However, also other extensions do not specify the metrics port, e.g. gardener/gardener-extension-provider-aws@f028217. For extensions with a webhook usually that port is explicitly exposed.

Do you want to also expose the metrics port (unlike some other extensions)?

@DockToFuture
Copy link
Member

I guess we need here to also set the metrics port on the svc.

We could also explicitly define the metrics port in the service. However, also other extensions do not specify the metrics port, e.g. gardener/gardener-extension-provider-aws@f028217. For extensions with a webhook usually that port is explicitly exposed.

Do you want to also expose the metrics port (unlike some other extensions)?

I looked at the calico extension and there we have explicitly exposed the port. It looked like there was something missing.

Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Mar 13, 2023
@gardener-robot gardener-robot removed the needs/review Needs review label Mar 13, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 13, 2023
@ScheererJ ScheererJ merged commit d4869c4 into gardener:master Mar 13, 2023
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev-productivity Developer productivity related (how to improve development) area/networking Networking related area/ops-productivity Operator productivity related (how to improve operations) area/security Security related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/s Size of pull request is small (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants