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
200 changes: 200 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,56 @@ spec:
- AMDEncryptedVirtualizationNestedPaging
type: string
type: object
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
Comment on lines +202 to +251
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

🌐 Web query:

In Kubernetes CRD x-kubernetes-validations, does has(self.someList) evaluate to true when the list field is present but empty ([]), and is minItems: 1 the recommended schema-level way to require at least one list element?

💡 Result:

  • Yes. In CRD CEL, has(self.someList) is a presence test (for JSON-like objects: “is this field/key defined?”), not a “non-empty” test—so if someList is present in the object as [], has(self.someList) evaluates to true. This aligns with CEL’s intent of unifying presence checks across protobuf and JSON-like types. [1][2]

    • If you need “present and non-empty” in CEL, use something like has(self.someList) && self.someList.size() > 0.
  • Yes. To require at least one list element at the schema level, minItems: 1 is the standard / recommended OpenAPI-schema validation for arrays (preferred over CEL when an OpenAPI validation exists). [3][4]

Sources: Kubernetes CEL docs [2], cel-go README (has macro semantics) [1], Kubernetes CRD validation best practices (prefer OpenAPI validations) [3], controller-tools / OpenAPI marker for MinItems=1 [4].


Enforce non-empty dedicatedHost array when affinity: DedicatedHost.

The has(self.dedicatedHost) check only validates field presence, so a config with affinity: DedicatedHost and dedicatedHost: [] passes validation despite being unusable. Add minItems: 1 to the array definition to reject empty lists at the schema boundary.

Suggested fix
                          dedicatedHost:
+                           minItems: 1
                            description: |-

Apply the same change at lines 1812–1861, 3362–3411, and 5112–5161.

📝 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
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
minItems: 1
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@data/data/install.openshift.io_installconfigs.yaml` around lines 202 - 251,
The schema allows affinity: DedicatedHost with an empty dedicatedHost array
because has(self.dedicatedHost) only checks presence; update the dedicatedHost
array definition (the dedicatedHost property under hostPlacement with type:
array and item schema containing id) to include minItems: 1 so empty lists are
rejected, and ensure the existing x-kubernetes-validations rule that references
self.dedicatedHost remains unchanged; apply the same minItems: 1 addition to the
other duplicated dedicatedHost array blocks referenced in the comment (the other
two occurrences).

iamProfile:
description: |-
IAMProfile is the name of the IAM instance profile to use for the machine.
Expand Down Expand Up @@ -1759,6 +1809,56 @@ spec:
- AMDEncryptedVirtualizationNestedPaging
type: string
type: object
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
iamProfile:
description: |-
IAMProfile is the name of the IAM instance profile to use for the machine.
Expand Down Expand Up @@ -3259,6 +3359,56 @@ spec:
- AMDEncryptedVirtualizationNestedPaging
type: string
type: object
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
iamProfile:
description: |-
IAMProfile is the name of the IAM instance profile to use for the machine.
Expand Down Expand Up @@ -4959,6 +5109,56 @@ spec:
- AMDEncryptedVirtualizationNestedPaging
type: string
type: object
hostPlacement:
description: |-
hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host
for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL),
and to gain visibility and control over instance placement on a physical server.
When omitted, the instance is not constrained to a dedicated host.
properties:
affinity:
description: |-
affinity specifies the affinity setting for the instance.
Allowed values are AnyAvailable and DedicatedHost.
When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set.
When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host.
enum:
- DedicatedHost
- AnyAvailable
type: string
dedicatedHost:
description: |-
dedicatedHost specifies the exact host that an instance should be restarted on if stopped.
dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise.
items:
description: DedicatedHost represents the configuration
for the usage of dedicated host.
properties:
id:
description: |-
id identifies the AWS Dedicated Host on which the instance must run.
The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f).
Must be exactly 19 characters in length.
maxLength: 19
minLength: 19
type: string
x-kubernetes-validations:
- message: hostID must start with 'h-' followed
by 17 lowercase hexadecimal characters (0-9
and a-f)
rule: self.matches('^h-[0-9a-f]{17}$')
required:
- id
type: object
type: array
required:
- affinity
type: object
x-kubernetes-validations:
- message: dedicatedHost is required when affinity is DedicatedHost,
and forbidden otherwise
rule: 'has(self.affinity) && self.affinity == ''DedicatedHost''
? has(self.dedicatedHost) : !has(self.dedicatedHost)'
iamProfile:
description: |-
IAMProfile is the name of the IAM instance profile to use for the machine.
Expand Down
47 changes: 47 additions & 0 deletions pkg/asset/cluster/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ func PreTerraform(ctx context.Context, clusterID string, installConfig *installc
return err
}

if err := tagSharedDedicatedHosts(ctx, clusterID, installConfig); err != nil {
return err
}

if err := tagSharedIAMRoles(ctx, clusterID, installConfig); err != nil {
return err
}
Expand Down Expand Up @@ -244,6 +248,49 @@ func tagSharedIAMProfiles(ctx context.Context, clusterID string, installConfig *
return nil
}

// tagSharedDedicatedHosts tags users BYO dedicated hosts so they are not destroyed by the Installer.
func tagSharedDedicatedHosts(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error {
var dhNames []string

// DH is only allowed to be created in "worker" pool at this time. No need to check default platform
for _, compute := range installConfig.Config.Compute {
if compute.Name == "worker" {
mpool := awstypes.MachinePool{}
mpool.Set(compute.Platform.AWS)
if mpool.HostPlacement != nil && mpool.HostPlacement.DedicatedHost != nil {
for _, name := range mpool.HostPlacement.DedicatedHost {
dhNames = append(dhNames, name.ID)
}
}
}
}

if len(dhNames) == 0 {
return nil
}

logrus.Debugf("Tagging shared dedicated hosts: %v", dhNames)

tagKey, tagValue := sharedTag(clusterID)

ec2Client, err := awsconfig.NewEC2Client(ctx, awsconfig.EndpointOptions{
Region: installConfig.Config.Platform.AWS.Region,
Endpoints: installConfig.Config.Platform.AWS.ServiceEndpoints,
})
if err != nil {
return fmt.Errorf("failed to create EC2 client: %w", err)
}

if _, err = ec2Client.CreateTags(ctx, &ec2.CreateTagsInput{
Resources: dhNames,
Tags: []ec2types.Tag{{Key: &tagKey, Value: &tagValue}},
}); err != nil {
return errors.Wrap(err, "could not add tags to subnets")
}

return nil
}

func sharedTag(clusterID string) (string, string) {
return fmt.Sprintf("kubernetes.io/cluster/%s", clusterID), "shared"
}
45 changes: 45 additions & 0 deletions pkg/asset/installconfig/aws/dedicatedhosts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package aws

import (
"context"
"fmt"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/ec2"
"github.com/sirupsen/logrus"
)

// Host holds metadata for a dedicated host.
type Host struct {
ID string
Zone string
}

// dedicatedHosts retrieves a list of dedicated hosts and returns them in a map keyed by the host ID.
func dedicatedHosts(ctx context.Context, client *ec2.Client) (map[string]Host, error) {
hostsByID := map[string]Host{}

paginator := ec2.NewDescribeHostsPaginator(client, &ec2.DescribeHostsInput{})
for paginator.HasMorePages() {
page, err := paginator.NextPage(ctx)
if err != nil {
return nil, fmt.Errorf("fetching dedicated hosts: %w", err)
}

for _, h := range page.Hosts {
id := aws.ToString(h.HostId)
if id == "" {
// Skip entries lacking an ID (should not happen)
continue
}

logrus.Debugf("Found dedicatd host: %s", id)
hostsByID[id] = Host{
ID: id,
Zone: aws.ToString(h.AvailabilityZone),
}
}
}

return hostsByID, nil
}
21 changes: 21 additions & 0 deletions pkg/asset/installconfig/aws/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Metadata struct {
vpc VPC
instanceTypes map[string]InstanceType

Hosts map[string]Host
Region string `json:"region,omitempty"`
ProvidedSubnets []typesaws.Subnet `json:"subnets,omitempty"`
Services []typesaws.ServiceEndpoint `json:"services,omitempty"`
Expand Down Expand Up @@ -369,3 +370,23 @@ func (m *Metadata) InstanceTypes(ctx context.Context) (map[string]InstanceType,

return m.instanceTypes, nil
}

// DedicatedHosts retrieves all hosts available for use to verify against this installation for configured region.
func (m *Metadata) DedicatedHosts(ctx context.Context) (map[string]Host, error) {
m.mutex.Lock()
defer m.mutex.Unlock()

if len(m.Hosts) == 0 {
client, err := m.EC2Client(ctx)
if err != nil {
return nil, err
}

m.Hosts, err = dedicatedHosts(ctx, client)
if err != nil {
return nil, fmt.Errorf("error listing dedicated hosts: %w", err)
}
}

return m.Hosts, nil
}
Loading