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

Move networkpolicies out of /contrib into /common (#2385) #2457

Merged
merged 8 commits into from
Apr 16, 2024
Merged

Move networkpolicies out of /contrib into /common (#2385) #2457

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
408b64d
Move networkpolicies out of /contrib into /common (#2385)
rawc0der May 10, 2023
c5b17fa
extend OWNERS file
rawc0der May 10, 2023
7819972
disable seldon NP
rawc0der Oct 7, 2023
04747c5
enable seldon NP
rawc0der Oct 11, 2023
af840eb
add rawc0der as Reviewer
rawc0der Oct 12, 2023
d37c1bb
Merge remote-tracking branch 'origin/master' into promote-netpols-common
rawc0der Jan 12, 2024
4ff6ea1
Remove rawc0der reviewer
rawc0der Jan 12, 2024
e699190
remove the duplicate resource
rawc0der Jan 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions contrib/networkpolicies/OWNERS → common/networkpolicies/OWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ approvers:
reviewers:
- juliusvonkohout
- kimwnasptd
- TobiasGoerke
0 contrib/networkpolicies/README.md → common/networkpolicies/README.md
View file
Open in desktop
File renamed without changes.
3 changes: 1 addition & 2 deletions contrib/networkpolicies/cache-server.yaml → ...on/networkpolicies/base/cache-server.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: cache-server
namespace: kubeflow
Expand All @@ -18,4 +18,3 @@ spec:
port: 8443
policyTypes:
- Ingress

2 changes: 1 addition & 1 deletion ...rib/networkpolicies/centraldashboard.yaml → ...etworkpolicies/base/centraldashboard.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: centraldashboard
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion ...olicies/default-allow-same-namespace.yaml → ...es/base/default-allow-same-namespace.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: default-allow-same-namespace
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion contrib/networkpolicies/jupyter-web-app.yaml → ...networkpolicies/base/jupyter-web-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: jupyter-web-app
namespace: kubeflow
Expand Down
8 changes: 4 additions & 4 deletions ...rib/networkpolicies/katib-controller.yaml → ...etworkpolicies/base/katib-controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: katib-controller
namespace: kubeflow
Expand All @@ -16,8 +16,8 @@ spec:
- ports: # webhook
- protocol: TCP
port: 8443
# - ports: # metrics
# - protocol: TCP
# port: 8080
# - ports: # metrics
# - protocol: TCP
# port: 8080
policyTypes:
- Ingress
2 changes: 1 addition & 1 deletion ...rib/networkpolicies/katib-db-manager.yaml → ...etworkpolicies/base/katib-db-manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: katib-db-manager
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion contrib/networkpolicies/katib-ui.yaml → common/networkpolicies/base/katib-ui.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: katib-ui
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion ...etworkpolicies/kserve-models-web-app.yaml → ...kpolicies/base/kserve-models-web-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: kserve-models-web-app
namespace: kubeflow
Expand Down
3 changes: 1 addition & 2 deletions contrib/networkpolicies/kserve.yaml → common/networkpolicies/base/kserve.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: kserve
namespace: kubeflow
Expand Down
0 contrib/networkpolicies/kustomization.yaml → ...n/networkpolicies/base/kustomization.yaml
View file
Open in desktop
File renamed without changes.
3 changes: 1 addition & 2 deletions contrib/networkpolicies/metadata-envoy.yaml → .../networkpolicies/base/metadata-envoy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: metatada-envoy
namespace: kubeflow
Expand All @@ -21,4 +21,3 @@ spec:
- podSelector: {}
policyTypes:
- Ingress

3 changes: 1 addition & 2 deletions ...networkpolicies/metadata-grpc-server.yaml → ...rkpolicies/base/metadata-grpc-server.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: metadata-grpc-server
namespace: kubeflow
Expand All @@ -21,4 +21,3 @@ spec:
- podSelector: {} # allow all pods from the same namespace
policyTypes:
- Ingress

4 changes: 2 additions & 2 deletions contrib/networkpolicies/minio.yaml → common/networkpolicies/base/minio.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: minio
namespace: kubeflow
Expand All @@ -9,7 +9,7 @@ spec:
- key: app
operator: In
values:
- minio # artifact storage
- minio # artifact storage
ingress:
- from:
- namespaceSelector:
Expand Down
2 changes: 1 addition & 1 deletion contrib/networkpolicies/ml-pipeline-ui.yaml → .../networkpolicies/base/ml-pipeline-ui.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: ml-pipeline-ui
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion contrib/networkpolicies/ml-pipeline.yaml → common/networkpolicies/base/ml-pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: ml-pipeline
namespace: kubeflow
Expand Down
4 changes: 2 additions & 2 deletions contrib/networkpolicies/poddefaults.yaml → common/networkpolicies/base/poddefaults.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: poddefaults
namespace: kubeflow
Expand All @@ -9,7 +9,7 @@ spec:
- key: app
operator: In
values:
- poddefaults # mutating webhook
- poddefaults # mutating webhook
# https://www.elastic.co/guide/en/cloud-on-k8s/1.1/k8s-webhook-network-policies.html
# The kubernetes api server must reach the webhook
ingress:
Expand Down
3 changes: 1 addition & 2 deletions contrib/networkpolicies/seldon.yaml → common/networkpolicies/base/seldon.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
Copy link
Member

Choose a reason for hiding this comment

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

@juliusvonkohout Seldon isn't installed by default, so does it make sense to add this policy by default?

Copy link
Member

Choose a reason for hiding this comment

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

That depends. We can also think about moving it to /contrib/seldon

metadata:
name: seldon
namespace: kubeflow
Expand All @@ -18,4 +18,3 @@ spec:
port: 4443
policyTypes:
- Ingress

2 changes: 1 addition & 1 deletion ...networkpolicies/tensorboards-web-app.yaml → ...rkpolicies/base/tensorboards-web-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: tensorboards-web-app
namespace: kubeflow
Expand Down
2 changes: 1 addition & 1 deletion contrib/networkpolicies/volumes-web-app.yaml → ...networkpolicies/base/volumes-web-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: volumes-web-app
namespace: kubeflow
Expand Down
1 change: 0 additions & 1 deletion contrib/networkpolicies/.gitkeep
View file
Open in desktop

This file was deleted.

3 changes: 3 additions & 0 deletions example/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ sortOptions:
- MutatingWebhookConfiguration
- ServiceAccount
- PodSecurityPolicy
- NetworkPolicy
- Role
- ClusterRole
- RoleBinding
Expand Down Expand Up @@ -49,6 +50,8 @@ resources:
- ../common/istio-1-16/cluster-local-gateway/base
# Kubeflow namespace
- ../common/kubeflow-namespace/base
# NetworkPolicies
- ../common/networkpolicies/base
Copy link
Member

Choose a reason for hiding this comment

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

@kimwnasptd @juliusvonkohout Are there any concerns with enabling network policies by default? Is there any concern around how impacts any additional components users deploy either from /contrib or their own?

This also would mean that any changes made by WG that do not align with current network policies would need to be addressed during the manifest testing time frame of the release. Is that something that Manifest WG wants to take on for all future releases as part of the release cycle?

Copy link
Member

Choose a reason for hiding this comment

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

I think there is a lot of discussion in #2385. It is more about how we want to do project governance and enforce security. I can do testing for that.

Copy link
Member

Choose a reason for hiding this comment

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

We should include automated tests with the PR if the netpol will be enabled by default, to test the current policies and future changes to the policies.

Copy link
Member

Choose a reason for hiding this comment

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

What exactly would you like to test?

Copy link
Member

Choose a reason for hiding this comment

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

Since we can't automatically trigger e2e right now, we can at least trigger all the component tests to ensure that it passes with netpol enabled.

Copy link
Member

Choose a reason for hiding this comment

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

Do you not think that it is a bit overkill to add the networkpolicies folder as path to 10 different GH actions? I would either use the e2e pipeline or make a simple test that just validates the networkpolicies with kustomize

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that just capturing kustomize build output is not enough, as Kustomize allows you to build invalid resources not following the OpenAPI spec definition.
Example:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: minio
  namespace: kubeflow
spec:
  podSelector:
    matchExpressions:
        - sexy_key: app
          operator: In
          values:
            - minio # artifact storage

With kustomize build common/networkpolicies/base there is no error captured, but when doing static validation against OpenAPI-based validation tools kubeval or kubectl-validate, an error is raised.

Kubeval:

$ kustomize build common/networkpolicies/base | kubeval                              (⎈ |kind-kind-dex-auth:default)
PASS - stdin contains a valid NetworkPolicy (kubeflow.cache-server)
WARN - stdin contains an invalid NetworkPolicy (kubeflow.minio) - key: key is required
....

Kubectl Validate:

$ kustomize build common/networkpolicies/base > netpol.yaml                          
$ kubectl-validate netpol.yaml                                                   
netpol.yaml...ERROR
spec.podSelector.matchExpressions[0].sexy_key: Invalid value: value provided for unknown field
Error: validation failed

@annajung my proposal is to consider using OpenAPI validation tools for testing manifest generation, aside from any other e2e and runtime checks.

  • Kubectl Validate is really just emerging, but seems to be under the right umbrella (Kubernetes SIGs)
  • Kubeval seems to be discontinued
  • any other promissing tools?

I can try to do a bit of hacking around with the GH Action workflows to test using the kubectl-validate route... let me know what's your thoughts on this.

Copy link
Member Author

@rawc0der rawc0der Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After checking the entire kustomize build example/ there are more errors surfaced:

$ kubectl-validate example.yaml                                                  

example.yaml...ERROR
Role.rbac.authorization.k8s.io "cert-manager-webhook:dynamic-serving" is invalid: metadata.name: Invalid value: "cert-manager-webhook:dynamic-serving": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Role.rbac.authorization.k8s.io "cert-manager-cainjector:leaderelection" is invalid: metadata.name: Invalid value: "cert-manager-cainjector:leaderelection": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Role.rbac.authorization.k8s.io "cert-manager:leaderelection" is invalid: metadata.name: Invalid value: "cert-manager:leaderelection": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
ClusterRole.rbac.authorization.k8s.io "cert-manager-controller-approve:cert-manager-io" is invalid: metadata.name: Invalid value: "cert-manager-controller-approve:cert-manager-io": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
ClusterRole.rbac.authorization.k8s.io "cert-manager-webhook:subjectaccessreviews" is invalid: metadata.name: Invalid value: "cert-manager-webhook:subjectaccessreviews": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
RoleBinding.rbac.authorization.k8s.io "cert-manager-webhook:dynamic-serving" is invalid: metadata.name: Invalid value: "cert-manager-webhook:dynamic-serving": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
RoleBinding.rbac.authorization.k8s.io "cert-manager-cainjector:leaderelection" is invalid: metadata.name: Invalid value: "cert-manager-cainjector:leaderelection": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
RoleBinding.rbac.authorization.k8s.io "cert-manager:leaderelection" is invalid: metadata.name: Invalid value: "cert-manager:leaderelection": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
ClusterRoleBinding.rbac.authorization.k8s.io "cert-manager-controller-approve:cert-manager-io" is invalid: metadata.name: Invalid value: "cert-manager-controller-approve:cert-manager-io": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
ClusterRoleBinding.rbac.authorization.k8s.io "cert-manager-webhook:subjectaccessreviews" is invalid: metadata.name: Invalid value: "cert-manager-webhook:subjectaccessreviews": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

Maybe deserves separate topic to cover checking multiple components. Adding the name to the file/resource where the errors are found would be useful as well, but I think also just the name of the Kubernetes resources is good for starters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unit test could look something like the following:
Screenshot 2023-10-19 at 17 25 51

But the problem is that for custom resources, operators etc, the CRDs needs to be passed as local folders for openAPI schemas extraction, so more work is needed for this.

failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: cert-manager.io/v1
failed to retrieve validator: failed to locate OpenAPI spec for GV: kubeflow.org/v1beta1
failed to retrieve validator: failed to locate OpenAPI spec for GV: metacontroller.k8s.io/v1alpha1
failed to retrieve validator: failed to locate OpenAPI spec for GV: networking.istio.io/v1alpha3
failed to retrieve validator: failed to locate OpenAPI spec for GV: networking.istio.io/v1alpha3

# Kubeflow Roles
- ../common/kubeflow-roles/base
# Kubeflow Istio Resources
Expand Down