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

Fix CRD installation #40

Merged
merged 1 commit into from
Sep 16, 2022
Merged

Fix CRD installation #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ If your repo has certain guidelines for contribution, put them here ahead of the
- [Kubernetes Contributor Guide](https://git.k8s.io/community/contributors/guide) - Main contributor documentation, or you can just jump directly to the [contributing section](https://git.k8s.io/community/contributors/guide#contributing)
- [Contributor Cheat Sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet) - Common resources for existing developers

### Installing the Admin Network Policy CRD

1) Clone the repo: `git clone https://github.com/kubernetes-sigs/network-policy-api.git`
2) Run `cd network-policy-api` && `make install`

## Mentorship

- [Mentoring Initiatives](https://git.k8s.io/community/mentoring) - We have a diverse set of mentorship programs available that are always looking for volunteers!
Expand Down
2 changes: 0 additions & 2 deletions apis/v1alpha1/shared_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ type AdminNetworkPolicyPort struct {
type Port struct {
// Protocol is the network protocol (TCP, UDP, or SCTP) which traffic must
// match. If not specified, this field defaults to TCP.
// +kubebuilder:default=TCP
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's possible I might need to use https://github.com/kubernetes/apiextensions-apiserver/blob/aabbdff63fe0a4e940e7cbe3e5188c3b6e4b63bf/pkg/apiserver/schema/validation.go#L49 (x-kubernetes-int-or-string: true) instead ? Would be good to check with an api expert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no I think we are good here.

Protocol v1.Protocol `json:"protocol"`

// Number defines a network port value.
Expand All @@ -81,7 +80,6 @@ type Port struct {
type PortRange struct {
// Protocol is the network protocol (TCP, UDP, or SCTP) which traffic must
// match. If not specified, this field defaults to TCP.
// +kubebuilder:default=TCP
Protocol v1.Protocol `json:"protocol,omitempty"`

// Start defines a network port that is the start of a port range, the Start
Expand Down
16 changes: 4 additions & 12 deletions config/crd/bases/policy.networking.k8s.io_adminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
Copy link
Member

Choose a reason for hiding this comment

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

Sou manually added this right? Does it get removed when you run make all? I wonder why this kube builder tag isn't working right :(

Copy link
Contributor Author

@tssurya tssurya Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't manually add it, that's the best part! not exactly sure how the default:TCP is populated, but it wasn't me. I simply did the mentioned changes in the types.go and ran make install and it seems to fill in this value.

Copy link
Member

Choose a reason for hiding this comment

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

Yep theres some legacy API funniness ensuring the default, see https://github.com/kubernetes/api/blob/master/networking/v1/types.go#L148 there's no explicit default in netpol either

This should be alright!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's probably that port.protocol already has TCP as its magic default somehow.

description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand All @@ -112,9 +110,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand Down Expand Up @@ -699,9 +695,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand All @@ -723,9 +717,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand Down
16 changes: 4 additions & 12 deletions config/crd/bases/policy.networking.k8s.io_baselineadminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand All @@ -109,9 +107,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand Down Expand Up @@ -691,9 +687,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand All @@ -715,9 +709,7 @@ spec:
minimum: 1
type: integer
protocol:
allOf:
- default: TCP
- default: TCP
default: TCP
description: Protocol is the network protocol (TCP,
UDP, or SCTP) which traffic must match. If not specified,
this field defaults to TCP.
Expand Down
4 changes: 4 additions & 0 deletions config/crd/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ patchesStrategicMerge:
#- patches/cainjection_in_adminnetworkpolicies.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch

# [API-APPROVED] To enable the CRD, we need to patch the api-approved annotation
- patches/apiapproved_annotation_in_baselineadminnetworkpolicies.yaml
- patches/apiapproved_annotation_in_adminnetworkpolicies.yaml

# the following config is for teaching kustomize how to do kustomization for CRDs.
configurations:
- kustomizeconfig.yaml
7 changes: 7 additions & 0 deletions config/crd/patches/apiapproved_annotation_in_adminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# The following patch adds the api-approved annotation to the CRD. See https://github.com/kubernetes/enhancements/pull/1111
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
api-approved.kubernetes.io: "https://github.com/kubernetes/enhancements/pull/2522"
name: adminnetworkpolicies.policy.networking.k8s.io
7 changes: 7 additions & 0 deletions config/crd/patches/apiapproved_annotation_in_baselineadminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# The following patch adds the api-approved annotation to the CRD. See https://github.com/kubernetes/enhancements/pull/1111
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
api-approved.kubernetes.io: "https://github.com/kubernetes/enhancements/pull/2522"
name: baselineadminnetworkpolicies.policy.networking.k8s.io