PCP-5519: Cherry pick HostResourceGroup upstream PR in palette fork #990

AmitSahastra · 2025-11-26T11:15:08Z

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

Release note:

…achine.Spec Signed-off-by: Pankaj Walke <[email protected]>

bulwark-spectrocloud

⚠️ GoSec scan found code issues:

G401: Use of weak cryptographic primitive, Severity: MEDIUM
- 1. File: /home/runner/_work/bulwark/bulwark/target-repo/pkg/cloud/services/eks/iam/iam.go:532:13
G505: Blocklisted import crypto/sha1: weak cryptographic primitive, Severity: MEDIUM
- 1. File: /home/runner/_work/bulwark/bulwark/target-repo/pkg/cloud/services/eks/iam/iam.go:22:2

Please review these findings and fix the issues before merging.

bulwark-spectrocloud

⚠️ GoVulnCheck scan found vulnerabilities:

GO-2025-3553
- Module: github.com/golang-jwt/jwt/v4
- Found in: v4.5.1
- Fixed in: v4.5.2
- Example Traces:
  1. pkg/rosa/client.go:51:70: rosa.NewOCMClient calls ocm.Build, which eventually calls authentication.Build
GO-2025-4123
- Module: github.com/dvsekhvalnov/jose2go
- Found in: v1.6.0
- Fixed in: v1.7.0
- Example Traces:
  1. pkg/rosa/client.go:51:70: rosa.NewOCMClient calls ocm.Build, which eventually calls keyring.Get

Please review these findings and fix the issues before merging.

bulwark-spectrocloud

⚠️ GoSec scan found code issues:

G401: Use of weak cryptographic primitive, Severity: MEDIUM
- 1. File: /home/runner/_work/bulwark/bulwark/target-repo/pkg/cloud/services/eks/iam/iam.go:532:13
G505: Blocklisted import crypto/sha1: weak cryptographic primitive, Severity: MEDIUM
- 1. File: /home/runner/_work/bulwark/bulwark/target-repo/pkg/cloud/services/eks/iam/iam.go:22:2

Please review these findings and fix the issues before merging.

Copilot

Pull request overview

This PR cherry-picks upstream changes related to Host Resource Group functionality into the Palette fork of the AWS Cluster API provider. The changes add support for dedicated host allocation options including static host IDs, host resource groups, and dynamic host allocation capabilities.

Key changes include:

Added new API fields for dedicated host allocation (HostID, HostResourceGroupArn, LicenseConfigurationArns, HostAffinity)
Implemented validation logic to ensure mutually exclusive host allocation options
Added support for capacity reservation preferences and CPU options for confidential computing

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/cloud/services/ec2/instances.go	Implements host allocation logic in instance creation and adds error handling for licensing issues
api/v1beta2/types.go	Defines new types for host allocation, capacity reservations, and CPU options
api/v1beta2/awsmachine_types.go	Adds host allocation fields to AWSMachineSpec
api/v1beta2/awsmachine_webhook.go	Implements validation for mutually exclusive host allocation options
api/v1beta2/awsmachine_webhook_test.go	Adds test cases for host allocation validation
api/v1beta2/awsmachinetemplate_webhook.go	Adds validation logic for host allocation in machine templates
api/v1beta2/awsmachinetemplate_webhook_test.go	Adds test cases for template validation
api/v1beta2/zz_generated.deepcopy.go	Auto-generated deep copy methods for new types
api/v1beta1/zz_generated.conversion.go	Auto-generated conversion warnings for v1beta1 compatibility
api/v1beta1/awsmachine_conversion.go	Implements conversion logic for new fields
api/v1beta1/awscluster_conversion.go	Implements conversion logic for bastion host fields
config/crd/bases/*.yaml	Updates CRD definitions with new fields and enhanced field descriptions

Comments suppressed due to low confidence (1)

api/v1beta2/awsmachinetemplate_webhook_test.go:1

Test case expects validation to pass when hostResourceGroupArn is specified without licenseConfigurationArns, but the validation logic at line 200-204 of awsmachinetemplate_webhook.go requires licenseConfigurationArns when hostResourceGroupArn is set. This test should expect wantErr: true.

/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T10:47:33Z

pkg/cloud/services/ec2/instances.go

+		//hostID, err := s.ensureDedicatedHostAllocation(context.Background(), scope)
+		//if err != nil {
+		//	return nil, errors.Wrap(err, "failed to allocate dedicated host")
+		//}
+		//input.HostID = aws.String(hostID)
+		//input.HostAffinity = aws.String("host")
+
+		//if scope.AWSMachine.Status.DedicatedHost == nil {
+		//	scope.AWSMachine.Status.DedicatedHost = &infrav1.DedicatedHostStatus{}
+		//}
+		//// Update machine status with allocated host ID
+		//scope.AWSMachine.Status.DedicatedHost.ID = &hostID


The commented-out implementation for dynamic host allocation should either be removed or uncommented. Dead code that's checked in reduces code clarity and maintainability.

Suggested change

//hostID, err := s.ensureDedicatedHostAllocation(context.Background(), scope)

//if err != nil {

// return nil, errors.Wrap(err, "failed to allocate dedicated host")

//}

//input.HostID = aws.String(hostID)

//input.HostAffinity = aws.String("host")

//if scope.AWSMachine.Status.DedicatedHost == nil {

// scope.AWSMachine.Status.DedicatedHost = &infrav1.DedicatedHostStatus{}

//}

//// Update machine status with allocated host ID

//scope.AWSMachine.Status.DedicatedHost.ID = &hostID

// Dynamic host allocation is enabled, but implementation is currently not provided.

Copilot · 2025-12-05T10:47:34Z

pkg/cloud/services/ec2/instances.go

+
+	input.CapacityReservationPreference = scope.AWSMachine.Spec.CapacityReservationPreference
+
+	//input.CPUOptions = scope.AWSMachine.Spec.CPUOptions


Remove commented-out code for CPUOptions assignment. Since CPUOptions is defined in the API types, either implement the functionality or remove the comment.

Suggested change

//input.CPUOptions = scope.AWSMachine.Spec.CPUOptions

Copilot · 2025-12-05T10:47:34Z

api/v1beta2/awsmachine_webhook_test.go

+					HostResourceGroupArn: aws.String("arn:aws:resource-groups:us-west-2:123456789012:group/test-group"),
+				},
+			},
+			wantErr: false,


Test case expects validation to pass when hostResourceGroupArn is specified without licenseConfigurationArns, but the validation logic at line 485-488 of awsmachine_webhook.go requires licenseConfigurationArns when hostResourceGroupArn is set. This test should expect wantErr: true.

Suggested change

wantErr: false,

wantErr: true,

spectro-prow · 2025-12-05T16:16:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AmitSahastra, snehala27
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

punkwalker added 2 commits November 25, 2025 17:41

Initial changes for supporting HostResourceGroupArn parameter in AWSM…

599e7ab

…achine.Spec Signed-off-by: Pankaj Walke <[email protected]>

Add licenseConfigurationArns field in AWSMAchineSpec

86b24cb

spectro-prow added the size/XL label Nov 26, 2025

bulwark-spectrocloud bot requested changes Nov 26, 2025

View reviewed changes

Host resource group arn additional files, make generated

9c15aef

spectro-prow added size/XXL and removed size/XL labels Nov 26, 2025

bulwark-spectrocloud bot requested changes Nov 26, 2025

View reviewed changes

AmitSahastra requested review from cPu1, Copilot and snehala27 December 5, 2025 10:46

Copilot AI reviewed Dec 5, 2025

View reviewed changes

snehala27 approved these changes Dec 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PCP-5519: Cherry pick HostResourceGroup upstream PR in palette fork #990

PCP-5519: Cherry pick HostResourceGroup upstream PR in palette fork #990

Uh oh!

AmitSahastra commented Nov 26, 2025

Uh oh!

bulwark-spectrocloud bot left a comment

Uh oh!

bulwark-spectrocloud bot left a comment

Uh oh!

bulwark-spectrocloud bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

spectro-prow commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		input.CapacityReservationPreference = scope.AWSMachine.Spec.CapacityReservationPreference

		//input.CPUOptions = scope.AWSMachine.Spec.CPUOptions

PCP-5519: Cherry pick HostResourceGroup upstream PR in palette fork #990

Are you sure you want to change the base?

PCP-5519: Cherry pick HostResourceGroup upstream PR in palette fork #990

Uh oh!

Conversation

AmitSahastra commented Nov 26, 2025

Uh oh!

bulwark-spectrocloud bot left a comment

Choose a reason for hiding this comment

Uh oh!

bulwark-spectrocloud bot left a comment

Choose a reason for hiding this comment

Uh oh!

bulwark-spectrocloud bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

spectro-prow commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants