OCM-8315 | feat: add additional allowed principals config for hcp clu…

…sters

Signed-off-by: Mike Ng <[email protected]>

cmd/create/cluster/cmd.go

@@ -180,9 +180,10 @@ var args struct {
 	tags []string
 
 	// Hypershift options:
-	hostedClusterEnabled bool
-	billingAccount       string
-	noCni                bool
+	hostedClusterEnabled        bool
+	billingAccount              string
+	noCni                       bool
+	additionalAllowedPrincipals []string
 
 	// Cluster Admin
 	createAdminUser      bool
@@ -455,6 +456,15 @@ func initFlags(cmd *cobra.Command) {
 		"A file contains a PEM-encoded X.509 certificate bundle that will be "+
 			"added to the nodes' trusted certificate store.")
 
+	flags.StringSliceVar(
+		&args.additionalAllowedPrincipals,
+		"additional-allowed-principals",
+		nil,
+		"A comma-separated list of additional allowed principal ARNs "+
+			"to be added to the Hosted Control Plane's VPC Endpoint Service to enable additional "+
+			"VPC Endpoint connection requests to be automatically accepted.",
+	)
+
 	flags.BoolVar(&args.enableCustomerManagedKey,
 		"enable-customer-managed-key",
 		false,
@@ -2852,6 +2862,31 @@ func run(cmd *cobra.Command, _ []string) {
 		os.Exit(1)
 	}
 
+	// Additional Allowed Principals
+	if cmd.Flags().Changed("additional-allowed-principals") && !isHostedCP {
+		r.Reporter.Errorf("Additional Allowed Principals is supported only for Hosted Control Planes")
+		os.Exit(1)
+	}
+	additionalAllowedPrincipals := args.additionalAllowedPrincipals
+	if isHostedCP && interactive.Enabled() {
+		aapInputs, err := interactive.GetString(interactive.Input{
+			Question: "Additional Allowed Principal ARNs",
+			Help:     cmd.Flags().Lookup("additional-allowed-principals").Usage,
+			Default:  strings.Join(additionalAllowedPrincipals, ","),
+		})
+		if err != nil {
+			r.Reporter.Errorf("Expected a valid value for Additional Allowed Principal ARNs: %s", err)
+			os.Exit(1)
+		}
+		additionalAllowedPrincipals = helper.HandleEmptyStringOnSlice(strings.Split(aapInputs, ","))
+	}
+	if len(additionalAllowedPrincipals) > 0 {
+		if err := roles.ValidateAdditionalAllowedPrincipals(additionalAllowedPrincipals); err != nil {
+			r.Reporter.Errorf(err.Error())
+			os.Exit(1)
+		}
+	}
+
 	// Audit Log Forwarding
 	auditLogRoleARN := args.AuditLogRoleARN
 
@@ -3100,6 +3135,7 @@ func run(cmd *cobra.Command, _ []string) {
 		AdditionalComputeSecurityGroupIds:      additionalComputeSecurityGroupIds,
 		AdditionalInfraSecurityGroupIds:        additionalInfraSecurityGroupIds,
 		AdditionalControlPlaneSecurityGroupIds: additionalControlPlaneSecurityGroupIds,
+		AdditionalAllowedPrincipals:            additionalAllowedPrincipals,
 	}
 
 	if httpTokens != "" {
@@ -3896,6 +3932,11 @@ func buildCommand(spec ocm.Spec, operatorRolesPrefix string,
 		command += " --no-cni"
 	}
 
+	if len(spec.AdditionalAllowedPrincipals) > 0 {
+		command += fmt.Sprintf(" --additional-allowed-principals %s",
+			strings.Join(spec.AdditionalAllowedPrincipals, ","))
+	}
+
 	for _, p := range properties {
 		command += fmt.Sprintf(" --properties \"%s\"", p)
 	}

cmd/describe/cluster/cmd.go

@@ -483,6 +483,12 @@ func run(cmd *cobra.Command, argv []string) {
 			str = fmt.Sprintf("%s"+
 				"Audit Log Role ARN:         %s\n", str, cluster.AWS().AuditLog().RoleArn())
 		}
+		if len(cluster.AWS().AdditionalAllowedPrincipals()) > 0 {
+			// Omitted the 'Allowed' due to formatting
+			str = fmt.Sprintf("%s"+
+				"Additional Principals:      %s\n", str,
+				strings.Join(cluster.AWS().AdditionalAllowedPrincipals(), ","))
+		}
 	}
 
 	if cluster.Status().State() == cmv1.ClusterStateError {

cmd/describe/cluster/cmd_test.go

@@ -23,6 +23,8 @@ var (
 		`{"displayName":"displayname","id":"bar","kind":"Cluster","name":"foo"}`)
 	expectClusterWithExternalAuthConfig = []byte(
 		`{"displayName":"displayname","external_auth_config":{"enabled":true},"kind":"Cluster"}`)
+	expectClusterWithAap = []byte(
+		`{"aws":{"additional_allowed_principals":["foobar"]},"displayName":"displayname","kind":"Cluster"}`)
 	expectClusterWithNameAndValueAndUpgradeInformation = []byte(
 		`{"displayName":"displayname","id":"bar","kind":"Cluster","name":"foo","scheduledUpgrade":{"nextRun":"` +
 			now.Format("2006-01-02 15:04 MST") + `","state":"` + state + `","version":"` +
@@ -32,9 +34,9 @@ var (
 			now.Format("2006-01-02 15:04 MST") + `","state":"` +
 			state + `","version":"` +
 			version + `"}}`)
-	clusterWithNameAndID, emptyCluster, clusterWithExternalAuthConfig *cmv1.Cluster
-	emptyUpgradePolicy, upgradePolicyWithVersionAndNextRun            *cmv1.UpgradePolicy
-	emptyUpgradeState, upgradePolicyWithState                         *cmv1.UpgradePolicyState
+	clusterWithNameAndID, emptyCluster, clusterWithExternalAuthConfig, clusterWithAap *cmv1.Cluster
+	emptyUpgradePolicy, upgradePolicyWithVersionAndNextRun                            *cmv1.UpgradePolicy
+	emptyUpgradeState, upgradePolicyWithState                                         *cmv1.UpgradePolicyState
 
 	berr error
 )
@@ -46,6 +48,9 @@ var _ = BeforeSuite(func() {
 	externalAuthConfig := cmv1.NewExternalAuthConfig().Enabled(true)
 	clusterWithExternalAuthConfig, berr = cmv1.NewCluster().ExternalAuthConfig(externalAuthConfig).Build()
 	Expect(berr).NotTo(HaveOccurred())
+	additionalAllowedPrincipals := cmv1.NewAWS().AdditionalAllowedPrincipals("foobar")
+	clusterWithAap, berr = cmv1.NewCluster().AWS(additionalAllowedPrincipals).Build()
+	Expect(berr).NotTo(HaveOccurred())
 	emptyUpgradePolicy, berr = cmv1.NewUpgradePolicy().Build()
 	Expect(berr).NotTo(HaveOccurred())
 	emptyUpgradeState, berr = cmv1.NewUpgradePolicyState().Build()
@@ -108,6 +113,11 @@ var _ = Describe("Cluster description", Ordered, func() {
 				func() *cmv1.Cluster { return clusterWithExternalAuthConfig },
 				func() *cmv1.UpgradePolicy { return emptyUpgradePolicy },
 				func() *cmv1.UpgradePolicyState { return nil }, expectClusterWithExternalAuthConfig, nil),
+
+			Entry("Prints cluster information with the additional allowed principals",
+				func() *cmv1.Cluster { return clusterWithAap },
+				func() *cmv1.UpgradePolicy { return emptyUpgradePolicy },
+				func() *cmv1.UpgradePolicyState { return nil }, expectClusterWithAap, nil),
 		)
 	})
 })

cmd/edit/cluster/cmd.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/openshift/rosa/pkg/aws"
 	"github.com/openshift/rosa/pkg/helper"
+	"github.com/openshift/rosa/pkg/helper/roles"
 	"github.com/openshift/rosa/pkg/input"
 	"github.com/openshift/rosa/pkg/interactive"
 	"github.com/openshift/rosa/pkg/interactive/confirm"
@@ -53,6 +54,9 @@ var args struct {
 
 	// Audit log forwarding
 	auditLogRoleARN string
+
+	// Other options
+	additionalAllowedPrincipals []string
 }
 
 var Cmd = &cobra.Command{
@@ -147,6 +151,15 @@ func init() {
 		"",
 		"The ARN of the role that is used to forward audit logs to AWS CloudWatch.",
 	)
+
+	flags.StringSliceVar(
+		&args.additionalAllowedPrincipals,
+		"additional-allowed-principals",
+		nil,
+		"A comma-separated list of additional allowed principal ARNs "+
+			"to be added to the Hosted Control Plane's VPC Endpoint Service to enable additional "+
+			"VPC Endpoint connection requests to be automatically accepted.",
+	)
 }
 
 func run(cmd *cobra.Command, _ []string) {
@@ -160,7 +173,7 @@ func run(cmd *cobra.Command, _ []string) {
 		changedFlags := false
 		for _, flag := range []string{"expiration-time", "expiration", "private",
 			"disable-workload-monitoring", "http-proxy", "https-proxy", "no-proxy",
-			"additional-trust-bundle-file", "audit-log-arn"} {
+			"additional-trust-bundle-file", "additional-allowed-principals", "audit-log-arn"} {
 			if cmd.Flags().Changed(flag) {
 				changedFlags = true
 			}
@@ -230,6 +243,11 @@ func run(cmd *cobra.Command, _ []string) {
 		os.Exit(1)
 	}
 
+	var additionalAllowedPrincipals []string
+	if cmd.Flags().Changed("additional-allowed-principals") {
+		additionalAllowedPrincipals = args.additionalAllowedPrincipals
+	}
+
 	var private *bool
 	var privateValue bool
 	if cmd.Flags().Changed("private") {
@@ -500,6 +518,47 @@ func run(cmd *cobra.Command, _ []string) {
 		}
 	}
 
+	/*******  AdditionalAllowedPrincipals *******/
+	updateAdditionalAllowedPrincipals := false
+	if additionalAllowedPrincipals != nil {
+		updateAdditionalAllowedPrincipals = true
+	}
+	if !updateAdditionalAllowedPrincipals && additionalAllowedPrincipals == nil &&
+		interactive.Enabled() {
+		updateAdditionalAllowedPrincipalsValue, err := interactive.GetBool(interactive.Input{
+			Question: "Update additional allowed principals",
+			Default:  updateAdditionalAllowedPrincipals,
+		})
+		if err != nil {
+			r.Reporter.Errorf("Expected a valid update-additional-allowed-principals value: %s", err)
+			os.Exit(1)
+		}
+		updateAdditionalAllowedPrincipals = updateAdditionalAllowedPrincipalsValue
+	}
+	if updateAdditionalAllowedPrincipals && interactive.Enabled() {
+		aapInputs, err := interactive.GetString(interactive.Input{
+			Question: "Additional Allowed Principal ARNs",
+			Help:     cmd.Flags().Lookup("additional-allowed-principals").Usage,
+			Default:  cluster.AWS().AdditionalAllowedPrincipals(),
+		})
+		if err != nil {
+			r.Reporter.Errorf("Expected a valid value for Additional Allowed Principal ARNs: %s", err)
+			os.Exit(1)
+		}
+		additionalAllowedPrincipals = helper.HandleEmptyStringOnSlice(strings.Split(aapInputs, ","))
+	}
+	if len(additionalAllowedPrincipals) > 0 {
+		if len(additionalAllowedPrincipals) == 1 &&
+			additionalAllowedPrincipals[0] == input.DoubleQuotesToRemove {
+			additionalAllowedPrincipals = []string{}
+		} else {
+			if err := roles.ValidateAdditionalAllowedPrincipals(additionalAllowedPrincipals); err != nil {
+				r.Reporter.Errorf(err.Error())
+				os.Exit(1)
+			}
+		}
+	}
+
 	// Audit Log Forwarding
 	auditLogRole, err := setAuditLogForwarding(r, cmd, cluster, args.auditLogRoleARN)
 	if err != nil {
@@ -549,6 +608,10 @@ func run(cmd *cobra.Command, _ []string) {
 		}
 	}
 
+	if additionalAllowedPrincipals != nil {
+		clusterConfig.AdditionalAllowedPrincipals = additionalAllowedPrincipals
+	}
+
 	if auditLogRole != nil {
 		clusterConfig.AuditLogRoleARN = new(string)
 		*clusterConfig.AuditLogRoleARN = *auditLogRole

pkg/helper/roles/helpers.go

@@ -281,3 +281,19 @@ func upgradeMissingOperatorRole(missingRoles map[string]*cmv1.STSOperator, clust
 	}
 	return nil
 }
+
+func ValidateAdditionalAllowedPrincipals(aapARNs []string) error {
+	duplicate, found := aws.HasDuplicates(aapARNs)
+	if found {
+		return fmt.Errorf("Invalid additional allowed principals list, duplicate key '%s' found",
+			duplicate)
+	}
+	for _, aap := range aapARNs {
+		err := aws.ARNValidator(aap)
+		if err != nil {
+			return fmt.Errorf("Expected valid ARNs for additional allowed principals list: %s",
+				err)
+		}
+	}
+	return nil
+}

pkg/helper/roles/helpers_test.go

@@ -0,0 +1,66 @@
+/*
+Copyright (c) 2024 Red Hat, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package roles
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestRolesHelper(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Roles Helper Suite")
+}
+
+var _ = Describe("Roles Helper", func() {
+	Context("Validate Additional Allowed Principals", func() {
+		It("should pass when valid ARNs", func() {
+			aapARNs := []string{
+				"arn:aws:iam::123456789012:role/role1",
+				"arn:aws:iam::123456789012:role/role2",
+			}
+			err := ValidateAdditionalAllowedPrincipals(aapARNs)
+			Expect(err).ShouldNot(HaveOccurred())
+		})
+		It("should error when containing duplicate ARNs", func() {
+			aapARNs := []string{
+				"arn:aws:iam::123456789012:role/role1",
+				"arn:aws:iam::123456789012:role/role2",
+				"arn:aws:iam::123456789012:role/role1",
+			}
+			err := ValidateAdditionalAllowedPrincipals(aapARNs)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal(
+				"Invalid additional allowed principals list, " +
+					"duplicate key 'arn:aws:iam::123456789012:role/role1' found"))
+		})
+
+		It("should error when contain invalid ARN", func() {
+			aapARNs := []string{
+				"arn:aws:iam::123456789012:role/role1",
+				"foobar",
+			}
+			err := ValidateAdditionalAllowedPrincipals(aapARNs)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring(
+				"Expected valid ARNs for additional allowed principals list"))
+		})
+
+	})
+})

pkg/ocm/clusters.go

@@ -141,9 +141,10 @@ type Spec struct {
 	AdditionalTrustBundle     *string
 
 	// HyperShift options:
-	Hypershift     Hypershift
-	BillingAccount string
-	NoCni          bool
+	Hypershift                  Hypershift
+	BillingAccount              string
+	NoCni                       bool
+	AdditionalAllowedPrincipals []string
 
 	// Audit Log Forwarding
 	AuditLogRoleARN *string
@@ -628,11 +629,16 @@ func (c *Client) UpdateCluster(clusterKey string, creator *aws.Creator, config S
 		clusterBuilder.Hypershift(hyperShiftBuilder)
 	}
 
-	// Edit audit log role arn
-	if config.AuditLogRoleARN != nil {
+	if config.AuditLogRoleARN != nil || config.AdditionalAllowedPrincipals != nil {
 		awsBuilder := cmv1.NewAWS()
-		auditLogBuiler := cmv1.NewAuditLog().RoleArn(*config.AuditLogRoleARN)
-		awsBuilder = awsBuilder.AuditLog(auditLogBuiler)
+		if config.AdditionalAllowedPrincipals != nil {
+			awsBuilder = awsBuilder.AdditionalAllowedPrincipals(config.AdditionalAllowedPrincipals...)
+		}
+		// Edit audit log role arn
+		if config.AuditLogRoleARN != nil {
+			auditLogBuiler := cmv1.NewAuditLog().RoleArn(*config.AuditLogRoleARN)
+			awsBuilder = awsBuilder.AuditLog(auditLogBuiler)
+		}
 		clusterBuilder.AWS(awsBuilder)
 	}
 
@@ -887,6 +893,10 @@ func (c *Client) createClusterSpec(config Spec) (*cmv1.Cluster, error) {
 		awsBuilder = awsBuilder.AdditionalControlPlaneSecurityGroupIds(config.AdditionalControlPlaneSecurityGroupIds...)
 	}
 
+	if len(config.AdditionalAllowedPrincipals) > 0 {
+		awsBuilder = awsBuilder.AdditionalAllowedPrincipals(config.AdditionalAllowedPrincipals...)
+	}
+
 	if config.SubnetIds != nil {
 		awsBuilder = awsBuilder.SubnetIDs(config.SubnetIds...)
 	}