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 kubebuilder validation for SecurityGroup resources #1710

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Fix kubebuilder validation for SecurityGroup resources #1710

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
21 changes: 12 additions & 9 deletions api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ type PortRange struct {
}

// SecurityGroup defines a VPC Security Group that should exist or be created within the specified VPC, with the specified Security Group Rules.
// +kubebuilder:validation:XValidation:rule="!has(self.id) && !has(self.name)",message="either an id or name must be specified"
// +kubebuilder:validation:XValidation:rule="has(self.id) || has(self.name)",message="either an id or name must be specified"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please help me understand this, How it would affect, User should set either id or name right?

https://playcel.undistro.io/?content=H4sIAAAAAAAAA3VRS0sDMRD%2BK%2BN6aAu1xdeltyJFkapFkSLsZbo73Q0kkyXJti7W%2F%2B5ku6hVSg4Z8mW%2Bx8xHkmPAZJKcwh05AuUhlHJxVQeIkJTwNn2Yg3Vw%2F%2FL0CGvrDIZRyil7ymqnQnPrbF1NUgZQ%2BQTOLy6vrlNOhgm9V468V5ZFYDyGJenMGoJgW5Gb2RwWGptC2jk%2FSVm%2BHL5FO8hiIZDDLKgNCcdq6j2ZlW6gslvxnAPxRjnLhjhEbpHVVrKgEEhNTrXIVoVyr2uNsQyzb3cwRy5qLAj6oj8YRSetmzNYSj6CxtautfaTCEqR7j69emqJ0RGCQLH2Kqc4q1%2BjHB6ZZceyiMzQe66516bYoK7xiDgWqNiHP7vqiGZd%2FtiXWa1JJic9di0MaCpNvjPmK%2BUwYml3SvT9g6WOVD6A3Q7%2BA4yGBsnnFwKmXNFAAgAA

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, a name or an id must be provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Karthik-K-N isn't this appropriate !has(self.id) && !has(self.name) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, Sometimes CEL expressions are confusing but the main logic is, The message will be displayed when the expression evaluates to false, In the above case if we set !has(self.id) && !has(self.name), it does not allow us to set either one of id or name.
Example when we set id !true && !false --> false && true --> false = either an id or name must be specified

More info: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules

type SecurityGroup struct {
// id of the Security Group.
// +optional
Expand Down Expand Up @@ -298,8 +298,10 @@ type SecurityGroup struct {

// SecurityGroupRule defines a VPC Security Group Rule for a specified Security Group.
// +kubebuilder:validation:XValidation:rule="(has(self.destination) && !has(self.source)) || (!has(self.destination) && has(self.source))",message="both destination and source cannot be provided"
// +kubebuilder:validation:XValidation:rule="has(self.destination) && self.direction == 'inbound'",message="destinationis not valid for SecurityGroupRuleDirectionInbound direction"
// +kubebuilder:validation:XValidation:rule="has(self.source) && self.direction == 'outbound'",message="source is not valid for SecurityGroupRuleDirectionOutbound direction"
// +kubebuilder:validation:XValidation:rule="self.direction == 'inbound' ? has(self.source) : true",message="source must be set for SecurityGroupRuleDirectionInbound direction"
// +kubebuilder:validation:XValidation:rule="self.direction == 'inbound' ? !has(self.destination) : true",message="destination is not valid for SecurityGroupRuleDirectionInbound direction"
// +kubebuilder:validation:XValidation:rule="self.direction == 'outbound' ? has(self.destination) : true",message="destination must be set for SecurityGroupRuleDirectionOutbound direction"
// +kubebuilder:validation:XValidation:rule="self.direction == 'outbound' ? !has(self.source) : true",message="source is not valid for SecurityGroupRuleDirectionOutbound direction"
type SecurityGroupRule struct {
// action defines whether to allow or deny traffic defined by the Security Group Rule.
// +required
Expand All @@ -326,10 +328,10 @@ type SecurityGroupRule struct {

// SecurityGroupRuleRemote defines a VPC Security Group Rule's remote details.
// The type of remote defines the additional remote details where are used for defining the remote.
// +kubebuilder:validation:XValidation:rule="self.remoteType == 'any' && (has(self.cidrSubnetName) || has(self.ip) || has(self.securityGroupName))",message="cidrSubnetName, ip, and securityGroupName are not valid for SecurityGroupRuleRemoteTypeAny remoteType"
// +kubebuilder:validation:XValidation:rule="has(self.cidrSubnetName) && self.remoteType != 'cidr'",message="cidrSubnetName is only valid for SecurityGroupRuleRemoteTypeCIDR remoteType"
// +kubebuilder:validation:XValidation:rule="has(self.ip) && self.remoteType != 'ip'",message="ip is only valid for SecurityGroupRuleRemoteTypeIP remoteType"
// +kubebuilder:validation:XValidation:rule="has(self.securityGroupName) && self.remoteType != 'sg'",message="securityGroupName is only valid for SecurityGroupRuleRemoteTypeSG remoteType"
// +kubebuilder:validation:XValidation:rule="self.remoteType == 'any' ? (!has(self.cidrSubnetName) && !has(self.ip) && !has(self.securityGroupName)) : true",message="cidrSubnetName, ip, and securityGroupName are not valid for SecurityGroupRuleRemoteTypeAny remoteType"
// +kubebuilder:validation:XValidation:rule="self.remoteType == 'cidr' ? (has(self.cidrSubnetName) && !has(self.ip) && !has(self.securityGroupName)) : true",message="only cidrSubnetName is valid for SecurityGroupRuleRemoteTypeCIDR remoteType"
// +kubebuilder:validation:XValidation:rule="self.remoteType == 'ip' ? (has(self.ip) && !has(self.cidrSubnetName) && !has(self.securityGroupName)) : true",message="only ip is valid for SecurityGroupRuleRemoteTypeIP remoteType"
// +kubebuilder:validation:XValidation:rule="self.remoteType == 'sg' ? (has(self.securityGroupName) && !has(self.cidrSubnetName) && !has(self.ip)) : true",message="only securityGroupName is valid for SecurityGroupRuleRemoteTypeSG remoteType"
type SecurityGroupRuleRemote struct {
// cidrSubnetName is the name of the VPC Subnet to retrieve the CIDR from, to use for the remote's destination/source.
// Only used when remoteType is SecurityGroupRuleRemoteTypeCIDR.
Expand All @@ -352,8 +354,9 @@ type SecurityGroupRuleRemote struct {
}

// SecurityGroupRulePrototype defines a VPC Security Group Rule's traffic specifics for a series of remotes (destinations or sources).
// +kubebuilder:validation:XValidation:rule="self.protocol != 'icmp' && (has(self.icmpCode) || has(self.icmpType))",message="icmpCode and icmpType are only supported for the ICMP protocol"
// +kubebuilder:validation:XValidation:rule="self.protocol == 'all' && has(self.portRange)",message="portRange is not valid for SecurityGroupRuleProtocolAll protocol"
// +kubebuilder:validation:XValidation:rule="self.protocol != 'icmp' ? (!has(self.icmpCode) && !has(self.icmpType)) : true",message="icmpCode and icmpType are only supported for SecurityGroupRuleProtocolIcmp protocol"
// +kubebuilder:validation:XValidation:rule="self.protocol == 'all' ? !has(self.portRange) : true",message="portRange is not valid for SecurityGroupRuleProtocolAll protocol"
// +kubebuilder:validation:XValidation:rule="self.protocol == 'icmp' ? !has(self.portRange) : true",message="portRange is not valid for SecurityGroupRuleProtocolIcmp protocol"
type SecurityGroupRulePrototype struct {
// icmpCode is the ICMP code for the Rule.
// Only used when Protocol is SecurityGroupProtocolICMP.
Expand Down