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

OCPBUGS-40906: Add encapsulation flag to IPsecConfig struct #1472

Merged
merged 1 commit into from
Dec 18, 2024
Merged

OCPBUGS-40906: Add encapsulation flag to IPsecConfig struct #1472

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
41 changes: 41 additions & 0 deletions openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28122,10 +28122,32 @@
"com.github.openshift.api.operator.v1.IPsecConfig": {
"type": "object",
"properties": {
"full": {
"description": "full defines configuration parameters for the IPsec `Full` mode. This is permitted only when mode is configured with `Full`, and forbidden otherwise.",
"$ref": "#/definitions/com.github.openshift.api.operator.v1.IPsecFullModeConfig"
},
"mode": {
"description": "mode defines the behaviour of the ipsec configuration within the platform. Valid values are `Disabled`, `External` and `Full`. When 'Disabled', ipsec will not be enabled at the node level. When 'External', ipsec is enabled on the node level but requires the user to configure the secure communication parameters. This mode is for external secure communications and the configuration can be done using the k8s-nmstate operator. When 'Full', ipsec is configured on the node level and inter-pod secure communication within the cluster is configured. Note with `Full`, if ipsec is desired for communication with external (to the cluster) entities (such as storage arrays), this is left to the user to configure.",
"type": "string"
}
},
"x-kubernetes-unions": [
{
"discriminator": "mode",
"fields-to-discriminateBy": {
"full": "Full"
}
}
]
},
"com.github.openshift.api.operator.v1.IPsecFullModeConfig": {
"description": "IPsecFullModeConfig defines configuration parameters for the IPsec `Full` mode.",
"type": "object",
"properties": {
"encapsulation": {
"description": "encapsulation option to configure libreswan on how inter-pod traffic across nodes are encapsulated to handle NAT traversal. When configured it uses UDP port 4500 for the encapsulation. Valid values are Always, Never, Auto and omitted. Always means enable UDP encapsulation regardless of whether NAT is detected. Disable means never enable UDP encapsulation even if NAT is present. Auto means enable UDP encapsulation based on the detection of NAT. When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time. The current default is Auto.",
"type": "string"
}
}
},
"com.github.openshift.api.operator.v1.IPv4GatewayConfig": {
Expand Down
38 changes: 38 additions & 0 deletions operator/v1/types_network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,8 @@ type HybridOverlayConfig struct {
}

// +kubebuilder:validation:XValidation:rule="self == oldSelf || has(self.mode)",message="ipsecConfig.mode is required"
// +kubebuilder:validation:XValidation:rule="has(self.mode) && self.mode == 'Full' ? true : !has(self.full)",message="full is forbidden when mode is not Full"
// +union
type IPsecConfig struct {
// mode defines the behaviour of the ipsec configuration within the platform.
// Valid values are `Disabled`, `External` and `Full`.
Expand All @@ -561,7 +563,43 @@ type IPsecConfig struct {
// this is left to the user to configure.
// +kubebuilder:validation:Enum=Disabled;External;Full
// +optional
// +unionDiscriminator
Mode IPsecMode `json:"mode,omitempty"`

// full defines configuration parameters for the IPsec `Full` mode.
// This is permitted only when mode is configured with `Full`,
// and forbidden otherwise.
// +unionMember,optional
// +optional
Full *IPsecFullModeConfig `json:"full,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs an explicit optional tag too please

Suggested change
Full *IPsecFullModeConfig `json:"full,omitempty"`
// +optional
Full *IPsecFullModeConfig `json:"full,omitempty"`

Copy link
Member

Choose a reason for hiding this comment

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

done.

}

type Encapsulation string

const (
// EncapsulationAlways always enable UDP encapsulation regardless of whether NAT is detected.
EncapsulationAlways = "Always"
// EncapsulationNever never enable UDP encapsulation even if NAT is present.
EncapsulationNever = "Never"
// EncapsulationAuto enable UDP encapsulation based on the detection of NAT.
EncapsulationAuto = "Auto"
)

// IPsecFullModeConfig defines configuration parameters for the IPsec `Full` mode.
// +kubebuilder:validation:MinProperties:=1
type IPsecFullModeConfig struct {
// encapsulation option to configure libreswan on how inter-pod traffic across nodes
// are encapsulated to handle NAT traversal. When configured it uses UDP port 4500
// for the encapsulation.
Comment on lines +591 to +593
Copy link

Choose a reason for hiding this comment

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

I think the purpose of this field is documented well in this GoDoc, but I would like to see more information outlined on:

  • Limitations of this field. I would recommend explicitly calling out the allowed values in the GoDoc and describing how each setting influences the behavior
  • As of writing, this field is optional and doesn't look to have a default - what does not setting this field mean for encapsulation?

Copy link
Contributor

Choose a reason for hiding this comment

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

As Bryce says, we need to add godoc explaining the options, this typically takes the form of

// Valid values are Force, Disable, Auto and omitted.
// Force means ... 
// Disable means ...
// Auto means ...
// When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time.
// The current default is ...

Also, given this is optional/omitempty you don't need "" as a valid value in the enum

Copy link
Member

Choose a reason for hiding this comment

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

sure, updated godoc with above info, hope it's fine now.

// Valid values are Always, Never, Auto and omitted.
// Always means enable UDP encapsulation regardless of whether NAT is detected.
// Disable means never enable UDP encapsulation even if NAT is present.
// Auto means enable UDP encapsulation based on the detection of NAT.
// When omitted, this means no opinion and the platform is left to choose a reasonable
// default, which is subject to change over time. The current default is Auto.
// +kubebuilder:validation:Enum:=Always;Never;Auto
// +optional
Encapsulation Encapsulation `json:"encapsulation,omitempty"`
}

type IPForwardingMode string
Expand Down
27 changes: 27 additions & 0 deletions operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,30 @@ spec:
ipsecConfig enables and configures IPsec for pods on the pod network within the
cluster.
properties:
full:
description: |-
full defines configuration parameters for the IPsec `Full` mode.
This is permitted only when mode is configured with `Full`,
and forbidden otherwise.
minProperties: 1
properties:
encapsulation:
description: |-
encapsulation option to configure libreswan on how inter-pod traffic across nodes
are encapsulated to handle NAT traversal. When configured it uses UDP port 4500
for the encapsulation.
Valid values are Always, Never, Auto and omitted.
Always means enable UDP encapsulation regardless of whether NAT is detected.
Disable means never enable UDP encapsulation even if NAT is present.
Auto means enable UDP encapsulation based on the detection of NAT.
When omitted, this means no opinion and the platform is left to choose a reasonable
default, which is subject to change over time. The current default is Auto.
enum:
- Always
- Never
- Auto
type: string
type: object
mode:
description: |-
mode defines the behaviour of the ipsec configuration within the platform.
Expand All @@ -431,6 +455,9 @@ spec:
x-kubernetes-validations:
- message: ipsecConfig.mode is required
rule: self == oldSelf || has(self.mode)
- message: full is forbidden when mode is not Full
rule: 'has(self.mode) && self.mode == ''Full'' ? true :
!has(self.full)'
ipv4:
description: |-
ipv4 allows users to configure IP settings for IPv4 connections. When ommitted,
Expand Down
27 changes: 27 additions & 0 deletions operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,30 @@ spec:
ipsecConfig enables and configures IPsec for pods on the pod network within the
cluster.
properties:
full:
description: |-
full defines configuration parameters for the IPsec `Full` mode.
This is permitted only when mode is configured with `Full`,
and forbidden otherwise.
minProperties: 1
properties:
encapsulation:
description: |-
encapsulation option to configure libreswan on how inter-pod traffic across nodes
are encapsulated to handle NAT traversal. When configured it uses UDP port 4500
for the encapsulation.
Valid values are Always, Never, Auto and omitted.
Always means enable UDP encapsulation regardless of whether NAT is detected.
Disable means never enable UDP encapsulation even if NAT is present.
Auto means enable UDP encapsulation based on the detection of NAT.
When omitted, this means no opinion and the platform is left to choose a reasonable
default, which is subject to change over time. The current default is Auto.
enum:
- Always
- Never
- Auto
type: string
type: object
mode:
description: |-
mode defines the behaviour of the ipsec configuration within the platform.
Expand All @@ -431,6 +455,9 @@ spec:
x-kubernetes-validations:
- message: ipsecConfig.mode is required
rule: self == oldSelf || has(self.mode)
- message: full is forbidden when mode is not Full
rule: 'has(self.mode) && self.mode == ''Full'' ? true :
!has(self.full)'
ipv4:
description: |-
ipv4 allows users to configure IP settings for IPv4 connections. When ommitted,
Expand Down
27 changes: 27 additions & 0 deletions ...or/v1/zz_generated.crd-manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,30 @@ spec:
ipsecConfig enables and configures IPsec for pods on the pod network within the
cluster.
properties:
full:
description: |-
full defines configuration parameters for the IPsec `Full` mode.
This is permitted only when mode is configured with `Full`,
and forbidden otherwise.
minProperties: 1
properties:
encapsulation:
description: |-
encapsulation option to configure libreswan on how inter-pod traffic across nodes
are encapsulated to handle NAT traversal. When configured it uses UDP port 4500
for the encapsulation.
Valid values are Always, Never, Auto and omitted.
Always means enable UDP encapsulation regardless of whether NAT is detected.
Disable means never enable UDP encapsulation even if NAT is present.
Auto means enable UDP encapsulation based on the detection of NAT.
When omitted, this means no opinion and the platform is left to choose a reasonable
default, which is subject to change over time. The current default is Auto.
enum:
- Always
- Never
- Auto
type: string
type: object
mode:
description: |-
mode defines the behaviour of the ipsec configuration within the platform.
Expand All @@ -431,6 +455,9 @@ spec:
x-kubernetes-validations:
- message: ipsecConfig.mode is required
rule: self == oldSelf || has(self.mode)
- message: full is forbidden when mode is not Full
rule: 'has(self.mode) && self.mode == ''Full'' ? true :
!has(self.full)'
ipv4:
description: |-
ipv4 allows users to configure IP settings for IPv4 connections. When ommitted,
Expand Down
27 changes: 27 additions & 0 deletions ...r/v1/zz_generated.crd-manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,30 @@ spec:
ipsecConfig enables and configures IPsec for pods on the pod network within the
cluster.
properties:
full:
description: |-
full defines configuration parameters for the IPsec `Full` mode.
This is permitted only when mode is configured with `Full`,
and forbidden otherwise.
minProperties: 1
properties:
encapsulation:
description: |-
encapsulation option to configure libreswan on how inter-pod traffic across nodes
are encapsulated to handle NAT traversal. When configured it uses UDP port 4500
for the encapsulation.
Valid values are Always, Never, Auto and omitted.
Always means enable UDP encapsulation regardless of whether NAT is detected.
Disable means never enable UDP encapsulation even if NAT is present.
Auto means enable UDP encapsulation based on the detection of NAT.
When omitted, this means no opinion and the platform is left to choose a reasonable
default, which is subject to change over time. The current default is Auto.
enum:
- Always
- Never
- Auto
type: string
type: object
mode:
description: |-
mode defines the behaviour of the ipsec configuration within the platform.
Expand All @@ -431,6 +455,9 @@ spec:
x-kubernetes-validations:
- message: ipsecConfig.mode is required
rule: self == oldSelf || has(self.mode)
- message: full is forbidden when mode is not Full
rule: 'has(self.mode) && self.mode == ''Full'' ? true :
!has(self.full)'
ipv4:
description: |-
ipv4 allows users to configure IP settings for IPv4 connections. When ommitted,
Expand Down
23 changes: 22 additions & 1 deletion operator/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading