Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 41 additions & 1 deletion machine/v1beta1/types_awsprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ const (

// HostPlacement is the type that will be used to configure the placement of AWS instances.
// +kubebuilder:validation:XValidation:rule="has(self.affinity) && self.affinity == 'DedicatedHost' ? has(self.dedicatedHost) : true",message="dedicatedHost is required when affinity is DedicatedHost, and optional otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.affinity) && has(self.dynamicHost) ? self.affinity == 'DynamicHost' : true",message="dynamicHost is only allowed when affinity is DynamicHost, and forbidden otherwise"
// +union
type HostPlacement struct {
// affinity specifies the affinity setting for the instance.
Expand Down Expand Up @@ -460,15 +461,54 @@ const (
HostAffinityDedicatedHost HostAffinity = "DedicatedHost"
)

// AllocationStrategy selects how a dedicated host is provided to the system for assigning to the instance.
// +kubebuilder:validation:Enum:=Provided;Dynamic
type AllocationStrategy string

const (
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
AllocationStrategyUserProvided AllocationStrategy = "Provided"

// AllocationStrategyDynamic specifies system to dynamically allocate a dedicated host to use for assignment to instances.
AllocationStrategyDynamic AllocationStrategy = "Dynamic"
)

// DedicatedHost represents the configuration for the usage of dedicated host.
// +kubebuilder:validation:XValidation:rule="self.allocationStrategy == 'Provided' ? has(self.id) : true",message="id is required when allocationStrategy is Provided, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.id) ? self.allocationStrategy == 'Provided' : true",message="id is only allowed when allocationStrategy is Provided"
// +kubebuilder:validation:XValidation:rule="has(self.dynamicHostAllocation) ? self.allocationStrategy == 'Dynamic' : true",message="dynamicHostAllocation is only allowed when allocationStrategy is Dynamic"
type DedicatedHost struct {
// allocationStrategy specifies if the dedicated host will be provided by the admin through the id field or if the host will be dynamically allocated.
// Valid values are Provided and Dynamic
// When AllocationStrategy is set to Provided, an ID of the dedicated host to assign must be provided.
// When AllocationStrategy is set to Dynamic, a dedicated host will be allocated and used to assign instances.
// When AllocationStrategy is set to Dynamic, and DynamicHostAllocation is provided, a dedicated host will be allocated and the tags in DynamicHostAllocation will be assigned to that host.
// +required
// +unionDiscriminator
AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`

// id identifies the AWS Dedicated Host on which the instance must run.
// The value must start with "h-" followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f).
// The use of 8 lowercase hexadecimal characters is for older legacy hosts that may not have been migrated to newer format.
// Must be either 10 or 19 characters in length.
// +kubebuilder:validation:XValidation:rule="self.matches('^h-([0-9a-f]{8}|[0-9a-f]{17})$')",message="hostID must start with 'h-' followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
// +kubebuilder:validation:MinLength=10
// +kubebuilder:validation:MaxLength=19
// +required
// +optional
// +unionMember
ID string `json:"id,omitempty"`

// dynamicHostAllocation specifies tags to apply to a dynamically allocated dedicated host.
// This field is mutually exclusive with id and always allocates exactly one host.
// +optional
// +unionMember
DynamicHostAllocation *DynamicHostAllocationSpec `json:"dynamicHostAllocation,omitempty"`
Comment on lines 476 to +505
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd machine && find . -name "*.go" | head -20

Repository: openshift/api

Length of output: 598


🏁 Script executed:

cd machine && rg -l "AllocationStrategy" --type go

Repository: openshift/api

Length of output: 131


🏁 Script executed:

cd machine && rg -A 5 "type AllocationStrategy" --type go

Repository: openshift/api

Length of output: 440


🏁 Script executed:

cd machine && rg -B 2 -A 10 "kubebuilder:default" v1beta1/ --type go | head -50

Repository: openshift/api

Length of output: 3437


🏁 Script executed:

cd machine && find . -name "*webhook*" -o -name "*defaulter*" | grep -i go

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && sed -n '476,510p' v1beta1/types_awsprovider.go

Repository: openshift/api

Length of output: 2675


🏁 Script executed:

cd machine && rg -B 5 -A 15 "type DedicatedHost struct" v1beta1/types_awsprovider.go

Repository: openshift/api

Length of output: 2055


🏁 Script executed:

cd machine && rg "DedicatedHost" v1beta1/ --type go | head -30

Repository: openshift/api

Length of output: 3319


🏁 Script executed:

cd machine && find . -name "*conversion*" -o -name "*webhook*" | grep -E "\.(go|yaml)$"

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && rg -l "func.*Default" --type go

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && rg -B 2 -A 20 "type AWSProviderSpec struct" v1beta1/types_awsprovider.go | head -60

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && rg -B 3 -A 3 "DedicatedHost \*DedicatedHost" v1beta1/types_awsprovider.go

Repository: openshift/api

Length of output: 329


🏁 Script executed:

cd machine && git log --oneline -20 -- v1beta1/types_awsprovider.go 2>/dev/null || echo "No git history available"

Repository: openshift/api

Length of output: 110


🏁 Script executed:

cd machine && rg -A 5 "AllocationStrategy" v1beta1/types_awsprovider.go | grep -E "const|type|="

Repository: openshift/api

Length of output: 630


🏁 Script executed:

cd machine && find . -name "*.yaml" -o -name "*test*.go" | xargs grep -l "DedicatedHost" 2>/dev/null | head -10

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && rg "omitempty" v1beta1/types_awsprovider.go | grep -i "allocationstrategy"

Repository: openshift/api

Length of output: 133


🏁 Script executed:

cd machine && git log -p -1 -- v1beta1/types_awsprovider.go 2>/dev/null | head -150 || echo "Git history not available"

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && find . -type f \( -name "*.yaml" -o -name "*.json" \) -exec grep -l "dedicatedHost\|DedicatedHost" {} \; 2>/dev/null | head -5

Repository: openshift/api

Length of output: 39


🏁 Script executed:

cd machine && rg "allocationStrategy" . --type yaml --type json 2>/dev/null || rg "allocationStrategy" . --type go | grep -v "^v1beta1/zz_" | head -20

Repository: openshift/api

Length of output: 1667


🏁 Script executed:

cd machine && rg -B 5 -A 5 "unionDiscriminator" v1beta1/ --type go | head -40

Repository: openshift/api

Length of output: 3693


Add default value to AllocationStrategy to preserve backward compatibility

The +required marker on AllocationStrategy without a default value will break existing manifests that only specify id without allocationStrategy. Since the XValidation rule enforces that allocationStrategy must be "Provided" when id is set, add a default to maintain backward compatibility.

Suggested fix
 	// +required
 	// +unionDiscriminator
+	// +kubebuilder:default:=Provided
 	AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// DedicatedHost represents the configuration for the usage of dedicated host.
// +kubebuilder:validation:XValidation:rule="self.allocationStrategy == 'Provided' ? has(self.id) : true",message="id is required when allocationStrategy is Provided, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.id) ? self.allocationStrategy == 'Provided' : true",message="id is only allowed when allocationStrategy is Provided"
// +kubebuilder:validation:XValidation:rule="has(self.dynamicHostAllocation) ? self.allocationStrategy == 'Dynamic' : true",message="dynamicHostAllocation is only allowed when allocationStrategy is Dynamic"
type DedicatedHost struct {
// allocationStrategy specifies if the dedicated host will be provided by the admin through the id field or if the host will be dynamically allocated.
// Valid values are Provided and Dynamic
// When AllocationStrategy is set to Provided, an ID of the dedicated host to assign must be provided.
// When AllocationStrategy is set to Dynamic, a dedicated host will be allocated and used to assign instances.
// When AllocationStrategy is set to Dynamic, and DynamicHostAllocation is provided, a dedicated host will be allocated and the tags in DynamicHostAllocation will be assigned to that host.
// +required
// +unionDiscriminator
AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`
// id identifies the AWS Dedicated Host on which the instance must run.
// The value must start with "h-" followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f).
// The use of 8 lowercase hexadecimal characters is for older legacy hosts that may not have been migrated to newer format.
// Must be either 10 or 19 characters in length.
// +kubebuilder:validation:XValidation:rule="self.matches('^h-([0-9a-f]{8}|[0-9a-f]{17})$')",message="hostID must start with 'h-' followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
// +kubebuilder:validation:MinLength=10
// +kubebuilder:validation:MaxLength=19
// +required
// +optional
// +unionMember
ID string `json:"id,omitempty"`
// dynamicHostAllocation specifies tags to apply to a dynamically allocated dedicated host.
// This field is mutually exclusive with id and always allocates exactly one host.
// +optional
// +unionMember
DynamicHostAllocation *DynamicHostAllocationSpec `json:"dynamicHostAllocation,omitempty"`
// DedicatedHost represents the configuration for the usage of dedicated host.
// +kubebuilder:validation:XValidation:rule="self.allocationStrategy == 'Provided' ? has(self.id) : true",message="id is required when allocationStrategy is Provided, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.id) ? self.allocationStrategy == 'Provided' : true",message="id is only allowed when allocationStrategy is Provided"
// +kubebuilder:validation:XValidation:rule="has(self.dynamicHostAllocation) ? self.allocationStrategy == 'Dynamic' : true",message="dynamicHostAllocation is only allowed when allocationStrategy is Dynamic"
type DedicatedHost struct {
// allocationStrategy specifies if the dedicated host will be provided by the admin through the id field or if the host will be dynamically allocated.
// Valid values are Provided and Dynamic
// When AllocationStrategy is set to Provided, an ID of the dedicated host to assign must be provided.
// When AllocationStrategy is set to Dynamic, a dedicated host will be allocated and used to assign instances.
// When AllocationStrategy is set to Dynamic, and DynamicHostAllocation is provided, a dedicated host will be allocated and the tags in DynamicHostAllocation will be assigned to that host.
// +required
// +unionDiscriminator
// +kubebuilder:default:=Provided
AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`
// id identifies the AWS Dedicated Host on which the instance must run.
// The value must start with "h-" followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f).
// The use of 8 lowercase hexadecimal characters is for older legacy hosts that may not have been migrated to newer format.
// Must be either 10 or 19 characters in length.
// +kubebuilder:validation:XValidation:rule="self.matches('^h-([0-9a-f]{8}|[0-9a-f]{17})$')",message="hostID must start with 'h-' followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
// +kubebuilder:validation:MinLength=10
// +kubebuilder:validation:MaxLength=19
// +optional
// +unionMember
ID string `json:"id,omitempty"`
// dynamicHostAllocation specifies tags to apply to a dynamically allocated dedicated host.
// This field is mutually exclusive with id and always allocates exactly one host.
// +optional
// +unionMember
DynamicHostAllocation *DynamicHostAllocationSpec `json:"dynamicHostAllocation,omitempty"`
🤖 Prompt for AI Agents
In `@machine/v1beta1/types_awsprovider.go` around lines 476 - 505, The
AllocationStrategy field in DedicatedHost must get a default of "Provided" to
preserve backward compatibility with manifests that only set id; add a
kubebuilder default marker (set default to Provided) on the AllocationStrategy
field (symbol AllocationStrategy in type DedicatedHost) so the XValidation rules
continue to allow legacy manifests that omit allocationStrategy when ID is
present, and ensure the struct tag/comment remains consistent with the new
default.

}

// DynamicHostAllocationSpec defines the configuration for dynamic dedicated host allocation.
// This specification always allocates exactly one dedicated host per machine.
type DynamicHostAllocationSpec struct {
// tags to apply to the allocated dedicated host.
// +optional
Tags map[string]string `json:"tags,omitempty"`
}
30 changes: 29 additions & 1 deletion machine/v1beta1/zz_generated.deepcopy.go

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

15 changes: 13 additions & 2 deletions machine/v1beta1/zz_generated.swagger_doc_generated.go

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

48 changes: 47 additions & 1 deletion openapi/generated_openapi/zz_generated.openapi.go

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

Loading