Miscellaneous code cleanup (#881)
Some checks are pending
Scorecard supply-chain security / Scorecard analysis (push) Waiting to run
Post / coverage (push) Waiting to run
Post / images (amd64) (push) Waiting to run
Post / images (arm64) (push) Waiting to run
Post / image manifest (push) Blocked by required conditions
Post / trigger clusteradm e2e (push) Blocked by required conditions

* extract constants and remove permissions

Signed-off-by: Alex <alexchan2988@gmail.com>

* Addressing miscellaneous code cleanup

Signed-off-by: Gaurav Jaswal <jaswalkiranavtar@gmail.com>

---------

Signed-off-by: Alex <alexchan2988@gmail.com>
Signed-off-by: Gaurav Jaswal <jaswalkiranavtar@gmail.com>
Co-authored-by: Alex <alexchan2988@gmail.com>
This commit is contained in:
jaswalkiranavtar
2025-03-12 20:45:19 -04:00
committed by GitHub
parent a5f3912a66
commit 57c78cd4de
29 changed files with 211 additions and 599 deletions

View File

@@ -64,7 +64,7 @@ func NewHubManagerOptions() *HubManagerOptions {
GCResourceList: []string{"addon.open-cluster-management.io/v1alpha1/managedclusteraddons",
"work.open-cluster-management.io/v1/manifestworks"},
ImportOption: importeroptions.New(),
EnabledRegistrationDrivers: []string{"csr"},
EnabledRegistrationDrivers: []string{commonhelpers.CSRAuthType},
}
}
@@ -170,7 +170,7 @@ func (m *HubManagerOptions) RunControllerManagerWithInformers(
var drivers []register.HubDriver
for _, enabledRegistrationDriver := range m.EnabledRegistrationDrivers {
switch enabledRegistrationDriver {
case "csr":
case commonhelpers.CSRAuthType:
autoApprovedCSRUsers := m.ClusterAutoApprovalUsers
if len(m.AutoApprovedCSRUsers) > 0 {
autoApprovedCSRUsers = m.AutoApprovedCSRUsers
@@ -180,7 +180,7 @@ func (m *HubManagerOptions) RunControllerManagerWithInformers(
return err
}
drivers = append(drivers, csrDriver)
case "awsirsa":
case commonhelpers.AwsIrsaAuthType:
awsIRSAHubDriver, err := awsirsa.NewAWSIRSAHubDriver(ctx, m.HubClusterArn, m.AutoApprovedARNPatterns, m.AwsResourceTags)
if err != nil {
return err

View File

@@ -64,7 +64,7 @@ func (c *AWSIRSADriver) BuildKubeConfigFromTemplate(kubeConfig *clientcmdapi.Con
kubeConfig.AuthInfos = map[string]*clientcmdapi.AuthInfo{register.DefaultKubeConfigAuth: {
Exec: &clientcmdapi.ExecConfig{
APIVersion: "client.authentication.k8s.io/v1beta1",
Command: "aws",
Command: "/awscli/dist/aws",
Args: []string{
"--region",
awsRegion,

View File

@@ -58,7 +58,7 @@ func TestBuildKubeconfig(t *testing.T) {
server: "https://127.0.0.1:6443",
AuthInfoExec: &clientcmdapi.ExecConfig{
APIVersion: "client.authentication.k8s.io/v1beta1",
Command: "aws",
Command: "/awscli/dist/aws",
Args: []string{
"--region",
"us-west-2",

View File

@@ -20,6 +20,7 @@ import (
clusterv1 "open-cluster-management.io/api/cluster/v1"
v1 "open-cluster-management.io/api/cluster/v1"
operatorv1 "open-cluster-management.io/api/operator/v1"
"open-cluster-management.io/ocm/manifests"
commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers"
@@ -27,8 +28,9 @@ import (
)
const (
errNoSuchEntity = "NoSuchEntity"
errEntityAlreadyExists = "EntityAlreadyExists"
errNoSuchEntity = "NoSuchEntity"
errEntityAlreadyExists = "EntityAlreadyExists"
trustPolicyTemplatePath = "managed-cluster-policy/TrustPolicy.tmpl"
)
type AWSIRSAHubDriver struct {
@@ -46,7 +48,7 @@ func (a *AWSIRSAHubDriver) Accept(cluster *clusterv1.ManagedCluster) bool {
return true
}
managedClusterArn := cluster.Annotations["agent.open-cluster-management.io/managed-cluster-arn"]
managedClusterArn := cluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterArn]
for _, p := range a.autoApprovedARNPatterns {
// Ensure the pattern matches the entire managed cluster ARN
if p.FindString(managedClusterArn) == managedClusterArn && len(managedClusterArn) > 0 {
@@ -59,8 +61,8 @@ func (a *AWSIRSAHubDriver) Accept(cluster *clusterv1.ManagedCluster) bool {
// Cleanup is run when the cluster is deleting or hubAcceptClient is set false
func (c *AWSIRSAHubDriver) Cleanup(ctx context.Context, managedCluster *clusterv1.ManagedCluster) error {
_, isManagedClusterIamRoleSuffixPresent :=
managedCluster.Annotations["agent.open-cluster-management.io/managed-cluster-iam-role-suffix"]
_, isManagedClusterArnPresent := managedCluster.Annotations["agent.open-cluster-management.io/managed-cluster-arn"]
managedCluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterIAMRoleSuffix]
_, isManagedClusterArnPresent := managedCluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterArn]
logger := klog.FromContext(ctx)
@@ -69,13 +71,13 @@ func (c *AWSIRSAHubDriver) Cleanup(ctx context.Context, managedCluster *clusterv
return nil
}
roleName, _, roleArn, policyArn, err := getRoleAndPolicyArn(ctx, managedCluster, c.cfg)
roleName, roleArn, err := getRoleNameAndArn(ctx, managedCluster, c.cfg)
if err != nil {
logger.V(4).Error(err, "Failed to getRoleAndPolicyArn")
logger.V(4).Error(err, "Failed to getRoleNameAndArn")
return err
}
err = deleteIAMRoleAndPolicy(ctx, c.cfg, roleName, policyArn)
err = deleteIAMRole(ctx, c.cfg, roleName)
if err != nil {
return err
}
@@ -95,8 +97,8 @@ func (c *AWSIRSAHubDriver) Run(_ context.Context, _ int) {
}
func (a *AWSIRSAHubDriver) allows(cluster *clusterv1.ManagedCluster) bool {
_, isManagedClusterArnPresent := cluster.Annotations["agent.open-cluster-management.io/managed-cluster-arn"]
_, isManagedClusterIAMRoleSuffixPresent := cluster.Annotations["agent.open-cluster-management.io/managed-cluster-iam-role-suffix"]
_, isManagedClusterArnPresent := cluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterArn]
_, isManagedClusterIAMRoleSuffixPresent := cluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterIAMRoleSuffix]
return isManagedClusterArnPresent && isManagedClusterIAMRoleSuffixPresent
}
@@ -123,7 +125,7 @@ func (a *AWSIRSAHubDriver) CreatePermissions(ctx context.Context, cluster *clust
}
// This function creates:
// 1. IAM Role and Policy in the hub cluster IAM
// 1. IAM Role and Trust Policy in the hub cluster IAM
// 2. Returns the hubClusterName and the roleArn to be used for Access Entry creation
func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCluster *v1.ManagedCluster, cfg aws.Config,
awsResourceTags []string) (string, string, error) {
@@ -139,15 +141,15 @@ func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCl
iamClient := iam.NewFromConfig(cfg)
managedClusterIamRoleSuffix, isManagedClusterIamRoleSuffixPresent :=
managedCluster.Annotations["agent.open-cluster-management.io/managed-cluster-iam-role-suffix"]
managedClusterArn, isManagedClusterArnPresent := managedCluster.Annotations["agent.open-cluster-management.io/managed-cluster-arn"]
managedCluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterIAMRoleSuffix]
managedClusterArn, isManagedClusterArnPresent := managedCluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterArn]
hubAccountId, hubClusterName = commonhelpers.GetAwsAccountIdAndClusterName(hubClusterArn)
managedClusterAccountId, managedClusterName = commonhelpers.GetAwsAccountIdAndClusterName(managedClusterArn)
roleName, policyName, roleArn, policyArn, err := getRoleAndPolicyArn(ctx, managedCluster, cfg)
roleName, roleArn, err := getRoleNameAndArn(ctx, managedCluster, cfg)
if err != nil {
logger.V(4).Error(err, "Failed to getRoleAndPolicyArn")
logger.V(4).Error(err, "Failed to getRoleNameAndArn")
return hubClusterName, roleArn, err
}
@@ -160,7 +162,6 @@ func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCl
return hubClusterName, roleArn, err
}
templateFiles := []string{"managed-cluster-policy/AccessPolicy.tmpl", "managed-cluster-policy/TrustPolicy.tmpl"}
data := map[string]interface{}{
"hubClusterArn": hubClusterArn,
"managedClusterAccountId": managedClusterAccountId,
@@ -169,9 +170,9 @@ func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCl
"hubClusterName": hubClusterName,
"managedClusterName": managedClusterName,
}
renderedTemplates, err := renderTemplates(templateFiles, data)
trustPolicy, err := renderTemplate(trustPolicyTemplatePath, data)
if err != nil {
logger.V(4).Error(err, "Failed to render templates while creating IAM role and policy for ManagedCluster", "ManagedCluster", managedClusterName)
logger.V(4).Error(err, "Failed to render template while creating IAM role and policy for ManagedCluster", "ManagedCluster", managedClusterName)
return hubClusterName, roleArn, err
}
@@ -184,7 +185,7 @@ func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCl
createRoleOutput, err = iamClient.CreateRole(ctx, &iam.CreateRoleInput{
RoleName: aws.String(roleName),
AssumeRolePolicyDocument: aws.String(renderedTemplates[1]),
AssumeRolePolicyDocument: aws.String(trustPolicy),
Tags: parsedTags,
})
if err != nil {
@@ -198,62 +199,29 @@ func createIAMRoleAndPolicy(ctx context.Context, hubClusterArn string, managedCl
} else {
logger.V(4).Info("Role created successfully for ManagedCluster", "IAMRole", *createRoleOutput.Role.Arn, "ManagedCluster", managedClusterName)
}
createPolicyResult, err := iamClient.CreatePolicy(ctx, &iam.CreatePolicyInput{
PolicyDocument: aws.String(renderedTemplates[0]),
PolicyName: aws.String(policyName),
})
if err != nil {
if !(strings.Contains(err.Error(), errEntityAlreadyExists)) {
logger.V(4).Error(err, "Failed to create IAM Policy for ManagedCluster", "IAMPolicy", policyName, "ManagedCluster", managedClusterName)
return hubClusterName, roleArn, err
} else {
logger.V(4).Info("Ignore IAM policy creation error for ManagedCluster as it already exists", "IAMPolicy", policyName, "ManagedCluster", managedClusterName)
}
} else {
logger.V(4).Info("Policy created successfully for ManagedCluster", "IAMPolicy", *createPolicyResult.Policy.Arn, "ManagedCluster", managedClusterName)
}
_, err = iamClient.AttachRolePolicy(ctx, &iam.AttachRolePolicyInput{
PolicyArn: aws.String(policyArn),
RoleName: aws.String(roleName),
})
if err != nil {
logger.V(4).Error(err, "Unable to attach policy to role for ManagedCluster",
"IAMPolicy", policyName, "IAMRole", roleName, "ManagedCluster", managedClusterName)
return hubClusterName, roleArn, err
} else {
logger.V(4).Info("Successfully attached IAM Policy to Role for ManagedCluster",
"IAMPolicy", policyName, "IAMRole", roleName, "ManagedCluster", managedClusterName)
}
}
return hubClusterName, roleArn, nil
}
func renderTemplates(argTemplates []string, data interface{}) (args []string, err error) {
func renderTemplate(argTemplate string, data interface{}) (args string, err error) {
var t *template.Template
var filebytes []byte
for _, arg := range argTemplates {
filebytes, err = manifests.ManagedClusterPolicyManifestFiles.ReadFile(arg)
if err != nil {
args = nil
return
}
contents := string(filebytes)
t, err = template.New(contents).Parse(contents)
if err != nil {
args = nil
return
}
buf := &bytes.Buffer{}
err = t.Execute(buf, data)
if err != nil {
args = nil
return
}
args = append(args, buf.String())
filebytes, err = manifests.ManagedClusterPolicyManifestFiles.ReadFile(argTemplate)
if err != nil {
return
}
contents := string(filebytes)
t, err = template.New(contents).Parse(contents)
if err != nil {
return
}
buf := &bytes.Buffer{}
err = t.Execute(buf, data)
if err != nil {
return
}
args = buf.String()
return
}
@@ -297,37 +265,12 @@ func createAccessEntry(ctx context.Context, eksClient *eks.Client, roleArn strin
return nil
}
func deleteIAMRoleAndPolicy(ctx context.Context, cfg aws.Config, roleName string, policyArn string) error {
func deleteIAMRole(ctx context.Context, cfg aws.Config, roleName string) error {
logger := klog.FromContext(ctx)
iamClient := iam.NewFromConfig(cfg)
_, err := iamClient.DetachRolePolicy(ctx, &iam.DetachRolePolicyInput{
RoleName: &roleName,
PolicyArn: &policyArn,
})
if err != nil {
if !strings.Contains(err.Error(), errNoSuchEntity) {
logger.V(4).Error(err, "Failed to detach Policy from Role", "Policy", policyArn, "Role", roleName)
return err
}
} else {
logger.V(4).Info("Policy detached successfully from Role", "Policy", policyArn, "Role", roleName)
}
_, err = iamClient.DeletePolicy(ctx, &iam.DeletePolicyInput{
PolicyArn: &policyArn,
})
if err != nil {
if !strings.Contains(err.Error(), errNoSuchEntity) {
logger.V(4).Error(err, "Failed to delete Policy", "Policy", policyArn)
return err
}
} else {
logger.V(4).Info("Policy deleted successfully", "Policy", policyArn)
}
_, err = iamClient.DeleteRole(ctx, &iam.DeleteRoleInput{
_, err := iamClient.DeleteRole(ctx, &iam.DeleteRoleInput{
RoleName: &roleName,
})
if err != nil {
@@ -342,24 +285,22 @@ func deleteIAMRoleAndPolicy(ctx context.Context, cfg aws.Config, roleName string
return nil
}
func getRoleAndPolicyArn(ctx context.Context, managedCluster *v1.ManagedCluster, cfg aws.Config) (string, string, string, string, error) {
func getRoleNameAndArn(ctx context.Context, managedCluster *v1.ManagedCluster, cfg aws.Config) (string, string, error) {
logger := klog.FromContext(ctx)
managedClusterIamRoleSuffix :=
managedCluster.Annotations["agent.open-cluster-management.io/managed-cluster-iam-role-suffix"]
managedCluster.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterIAMRoleSuffix]
roleName := fmt.Sprintf("ocm-hub-%s", managedClusterIamRoleSuffix)
policyName := roleName
creds, err := cfg.Credentials.Retrieve(ctx)
if err != nil {
logger.V(4).Error(err, "Failed to get IAM Credentials")
return "", "", "", "", err
return "", "", err
}
awsAccountId := creds.AccountID
roleArn := fmt.Sprintf("arn:aws:iam::%s:role/%s", awsAccountId, roleName)
policyArn := fmt.Sprintf("arn:aws:iam::%s:policy/%s", awsAccountId, policyName)
return roleName, policyName, roleArn, policyArn, err
return roleName, roleArn, err
}
func deleteAccessEntry(ctx context.Context, eksClient *eks.Client, roleArn string, hubClusterName string) error {

View File

@@ -20,6 +20,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "open-cluster-management.io/api/cluster/v1"
operatorv1 "open-cluster-management.io/api/operator/v1"
"open-cluster-management.io/ocm/manifests"
commonhelper "open-cluster-management.io/ocm/pkg/common/helpers"
@@ -38,8 +39,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster1",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -51,8 +52,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster2",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster2",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster2",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -64,8 +65,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster3",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-1:123456789012:cluster/managed-cluster3",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-1:123456789012:cluster/managed-cluster3",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -77,8 +78,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster4",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:999999999999:cluster/managed-cluster4",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:999999999999:cluster/managed-cluster4",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -90,8 +91,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster5",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "XXXXXXarn:aws:eks:us-west-2:123456789012:cluster/managed-cluster5",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "XXXXXXarn:aws:eks:us-west-2:123456789012:cluster/managed-cluster5",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -103,8 +104,8 @@ func TestAccept(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster6",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -151,8 +152,7 @@ func TestNewDriverValidation(t *testing.T) {
}
}
func TestRenderTemplates(t *testing.T) {
templateFiles := []string{"managed-cluster-policy/AccessPolicy.tmpl", "managed-cluster-policy/TrustPolicy.tmpl"}
func TestRenderTemplate(t *testing.T) {
data := map[string]interface{}{
"hubClusterArn": "arn:aws:iam::123456789012:cluster/hub-cluster",
"managedClusterAccountId": "123456789013",
@@ -167,17 +167,9 @@ func TestRenderTemplates(t *testing.T) {
data["managedClusterAccountId"].(string),
data["managedClusterName"].(string),
)
renderedTemplates, _ := renderTemplates(templateFiles, data)
trustPolicy, _ := renderTemplate(trustPolicyTemplatePath, data)
APfilebuf, APerr := manifests.ManagedClusterPolicyManifestFiles.ReadFile("managed-cluster-policy/AccessPolicy.tmpl")
if APerr != nil {
t.Errorf("Templates not rendered as expected")
return
}
contents := string(APfilebuf)
AccessPolicy := strings.Replace(contents, "{{.hubClusterArn}}", data["hubClusterArn"].(string), 1)
TPfilebuf, TPerr := manifests.ManagedClusterPolicyManifestFiles.ReadFile("managed-cluster-policy/TrustPolicy.tmpl")
TPfilebuf, TPerr := manifests.ManagedClusterPolicyManifestFiles.ReadFile(trustPolicyTemplatePath)
if TPerr != nil {
t.Errorf("Templates not rendered as expected")
return
@@ -193,17 +185,7 @@ func TestRenderTemplates(t *testing.T) {
TrustPolicy := replacer.Replace(contentstrust)
if len(renderedTemplates) != 2 {
t.Errorf("Templates not rendered as expected")
return
}
if renderedTemplates[0] != AccessPolicy {
t.Errorf("AccessPolicy not rendered as expected")
return
}
if renderedTemplates[1] != TrustPolicy {
if trustPolicy != TrustPolicy {
t.Errorf("TrustPolicy not rendered as expected")
return
}
@@ -223,20 +205,20 @@ func TestDeleteIAMRoleAndPolicy(t *testing.T) {
wantErr bool
}{
{
name: "test delete IAM Role and policy",
name: "test delete IAM Role",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: mockSuccessfulDeletionBehaviour,
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
},
{
name: "test delete IAM Role and policy with NoSuchEntity in DeleteRole",
name: "test delete IAM Role with NoSuchEntity in DeleteRole",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
@@ -247,73 +229,7 @@ func TestDeleteIAMRoleAndPolicy(t *testing.T) {
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"DeleteRoleOrDeletePolicyOrDetachPolicyMock3",
func(ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
if middleware.GetOperationName(ctx) == "DetachRolePolicy" {
return middleware.FinalizeOutput{
Result: &iam.DetachRolePolicyOutput{},
}, middleware.Metadata{}, fmt.Errorf("failed to detach IAM policy from role, NoSuchEntity")
}
return next.HandleFinalize(ctx, input)
},
),
middleware.Before,
)
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
},
{
name: "test delete IAM Role and policy with NoSuchEntity in DeletePolicy",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
err := mockSuccessfulDeletionBehaviour(stack)
if err != nil {
return err
}
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"DeleteRoleOrDeletePolicyOrDetachPolicyMock3",
func(ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
if middleware.GetOperationName(ctx) == "DeletePolicy" {
return middleware.FinalizeOutput{
Result: nil,
}, middleware.Metadata{}, fmt.Errorf("failed to delete IAM policy, NoSuchEntity")
}
return next.HandleFinalize(ctx, input)
},
),
middleware.Before,
)
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
},
{
name: "test delete IAM Role and policy with NoSuchEntity in DeleteRole",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
err := mockSuccessfulDeletionBehaviour(stack)
if err != nil {
return err
}
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"DeleteRoleOrDeletePolicyOrDetachPolicyMock3",
"DeleteRoleMock3",
func(ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
if middleware.GetOperationName(ctx) == "DeleteRole" {
return middleware.FinalizeOutput{
@@ -328,8 +244,8 @@ func TestDeleteIAMRoleAndPolicy(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
@@ -353,12 +269,12 @@ func TestDeleteIAMRoleAndPolicy(t *testing.T) {
managedCluster := testinghelpers.NewManagedCluster()
managedCluster.Annotations = tt.managedClusterAnnotations
roleName, _, _, policyArn, err := getRoleAndPolicyArn(tt.args.ctx, managedCluster, cfg)
roleName, _, err := getRoleNameAndArn(tt.args.ctx, managedCluster, cfg)
if err != nil {
t.Errorf("Error getting role and policy Arn.")
t.Errorf("Error getting role name")
return
}
err = deleteIAMRoleAndPolicy(tt.args.ctx, cfg, roleName, policyArn)
err = deleteIAMRole(tt.args.ctx, cfg, roleName)
if (err != nil) != tt.wantErr {
t.Errorf("error = %#v, wantErr %#v", err, tt.wantErr)
return
@@ -546,8 +462,8 @@ func TestCleanup(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "managed-cluster",
Annotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1",
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "7f8141296c75f2871e3d030f85c35692",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "7f8141296c75f2871e3d030f85c35692",
},
},
},
@@ -588,7 +504,7 @@ func TestCleanup(t *testing.T) {
}
}
func TestCreateIAMRoleAndPolicy(t *testing.T) {
func TestCreateIAMRole(t *testing.T) {
type args struct {
ctx context.Context
withAPIOptionsFunc func(*middleware.Stack) error
@@ -602,13 +518,13 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
wantErr bool
}{
{
name: "test create IAM Role and policy",
name: "test create IAM Role",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"CreateRoleOrCreatePolicyOrAttachPolicyMock",
"CreateRoleMock",
func(ctx context.Context, input middleware.FinalizeInput, handler middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
operationName := middleware.GetOperationName(ctx)
if operationName == "CreateRole" {
@@ -620,20 +536,6 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: &iam.CreatePolicyOutput{Policy: &iamtypes.Policy{
PolicyName: aws.String("TestPolicy"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "AttachRolePolicy" {
return middleware.FinalizeOutput{
Result: &iam.AttachRolePolicyOutput{},
}, middleware.Metadata{}, nil
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
@@ -642,8 +544,8 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
@@ -667,20 +569,6 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: &iam.CreatePolicyOutput{Policy: &iamtypes.Policy{
PolicyName: aws.String("TestPolicy"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "AttachRolePolicy" {
return middleware.FinalizeOutput{
Result: &iam.AttachRolePolicyOutput{},
}, middleware.Metadata{}, nil
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
@@ -689,14 +577,14 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "test",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "test",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: fmt.Errorf("HubClusterARN provided during join by ManagedCluster spoke-cluster is different from the current hub cluster"),
wantErr: true,
},
{
name: "test create IAM Role and policy with EntityAlreadyExists in CreateRole",
name: "test create IAM Role with EntityAlreadyExists in CreateRole",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
@@ -710,29 +598,6 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
Result: nil,
}, middleware.Metadata{}, fmt.Errorf("failed to create IAM role, EntityAlreadyExists")
}
if operationName == "GetRole" {
return middleware.FinalizeOutput{
Result: &iam.GetRoleOutput{Role: &iamtypes.Role{
RoleName: aws.String("TestRole"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestRole"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: &iam.CreatePolicyOutput{Policy: &iamtypes.Policy{
PolicyName: aws.String("TestPolicy"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "AttachRolePolicy" {
return middleware.FinalizeOutput{
Result: &iam.AttachRolePolicyOutput{},
}, middleware.Metadata{}, nil
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
@@ -741,14 +606,14 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
},
{
name: "test create IAM Role and policy with error in CreateRole",
name: "test create IAM Role with error in CreateRole",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
@@ -770,156 +635,12 @@ func TestCreateIAMRoleAndPolicy(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: fmt.Errorf("operation error IAM: CreateRole, failed to create IAM role"),
wantErr: true,
},
{
name: "test create IAM Role and policy with EntityAlreadyExists in CreatePolicy",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"CreatePolicyEntityAlreadyExistsMock",
func(ctx context.Context, input middleware.FinalizeInput, handler middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
operationName := middleware.GetOperationName(ctx)
if operationName == "CreateRole" {
return middleware.FinalizeOutput{
Result: &iam.CreateRoleOutput{Role: &iamtypes.Role{
RoleName: aws.String("TestRole"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestRole"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: nil,
}, middleware.Metadata{}, fmt.Errorf("failed to create IAM policy, EntityAlreadyExists")
}
if operationName == "ListPolicies" {
policies := []iamtypes.Policy{
{
PolicyName: aws.String("TestPolicy1"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy1"),
},
// You can add more policies here if needed
{
PolicyName: aws.String("ocm-hub-960c4e56c25ba0b571ddcdaa7edc943e"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy2"),
},
}
return middleware.FinalizeOutput{
Result: &iam.ListPoliciesOutput{Policies: policies},
}, middleware.Metadata{}, nil
}
if operationName == "AttachRolePolicy" {
return middleware.FinalizeOutput{
Result: &iam.AttachRolePolicyOutput{},
}, middleware.Metadata{}, nil
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
middleware.Before,
)
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
},
{
name: "test create IAM Role and policy with error in CreatePolicy",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"CreatePolicyErrorMock",
func(ctx context.Context, input middleware.FinalizeInput, handler middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
operationName := middleware.GetOperationName(ctx)
if operationName == "CreateRole" {
return middleware.FinalizeOutput{
Result: &iam.CreateRoleOutput{Role: &iamtypes.Role{
RoleName: aws.String("TestRole"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestRole"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: nil,
}, middleware.Metadata{}, fmt.Errorf("failed to create IAM policy")
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
middleware.Before,
)
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: fmt.Errorf("operation error IAM: CreatePolicy, failed to create IAM policy"),
wantErr: true,
},
{
name: "test create IAM Role and policy with error in AttachRolePolicy",
args: args{
ctx: context.Background(),
withAPIOptionsFunc: func(stack *middleware.Stack) error {
return stack.Finalize.Add(
middleware.FinalizeMiddlewareFunc(
"AttachRolePolicyErrorMock",
func(ctx context.Context, input middleware.FinalizeInput, handler middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {
operationName := middleware.GetOperationName(ctx)
if operationName == "CreateRole" {
return middleware.FinalizeOutput{
Result: &iam.CreateRoleOutput{Role: &iamtypes.Role{
RoleName: aws.String("TestRole"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestRole"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "CreatePolicy" {
return middleware.FinalizeOutput{
Result: &iam.CreatePolicyOutput{Policy: &iamtypes.Policy{
PolicyName: aws.String("TestPolicy"),
Arn: aws.String("arn:aws:iam::123456789012:role/TestPolicy"),
},
},
}, middleware.Metadata{}, nil
}
if operationName == "AttachRolePolicy" {
return middleware.FinalizeOutput{
Result: nil,
}, middleware.Metadata{}, fmt.Errorf("failed to attach policy to role")
}
return middleware.FinalizeOutput{}, middleware.Metadata{}, nil
},
),
middleware.Before,
)
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: fmt.Errorf("operation error IAM: AttachRolePolicy, failed to attach policy to role"),
wantErr: true,
},
}
for _, tt := range cases {
@@ -993,8 +714,8 @@ func TestCreateAccessEntries(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: nil,
wantErr: false,
@@ -1022,8 +743,8 @@ func TestCreateAccessEntries(t *testing.T) {
},
},
managedClusterAnnotations: map[string]string{
"agent.open-cluster-management.io/managed-cluster-iam-role-suffix": "960c4e56c25ba0b571ddcdaa7edc943e",
"agent.open-cluster-management.io/managed-cluster-arn": "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterIAMRoleSuffix: "960c4e56c25ba0b571ddcdaa7edc943e",
operatorv1.ClusterAnnotationsKeyPrefix + "/" + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/spoke-cluster",
},
want: fmt.Errorf("operation error EKS: CreateAccessEntry, failed to create access entry"),
wantErr: true,

View File

@@ -9,6 +9,7 @@ import (
ocmfeature "open-cluster-management.io/api/feature"
commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers"
"open-cluster-management.io/ocm/pkg/features"
"open-cluster-management.io/ocm/pkg/registration/helpers"
)
@@ -119,7 +120,7 @@ func (o *SpokeAgentOptions) Validate() error {
return errors.New("client certificate expiration seconds must greater or qual to 3600")
}
if (o.RegistrationAuth == AwsIrsaAuthType) && (o.HubClusterArn == "") {
if (o.RegistrationAuth == commonhelpers.AwsIrsaAuthType) && (o.HubClusterArn == "") {
return errors.New("EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa")
}

View File

@@ -43,8 +43,6 @@ import (
// TODO if we register the lease informer to the lease controller, we need to increase this time
var AddOnLeaseControllerSyncInterval = 30 * time.Second
const AwsIrsaAuthType = "awsirsa"
type SpokeAgentConfig struct {
agentOptions *commonoptions.AgentOptions
registrationOption *SpokeAgentOptions
@@ -190,7 +188,7 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context,
// initiate registration driver
var registerDriver register.RegisterDriver
if o.registrationOption.RegistrationAuth == AwsIrsaAuthType {
if o.registrationOption.RegistrationAuth == helpers.AwsIrsaAuthType {
registerDriver = awsIrsa.NewAWSIRSADriver(o.registrationOption.ManagedClusterArn,
o.registrationOption.ManagedClusterRoleSuffix,
o.registrationOption.HubClusterArn,
@@ -570,7 +568,7 @@ func (o *SpokeAgentConfig) newRestirationAuthOption(
clusterInformers clusterv1informers.SharedInformerFactory,
clusterClient clusterv1client.Interface,
) (any, error) {
if o.registrationOption.RegistrationAuth == AwsIrsaAuthType {
if o.registrationOption.RegistrationAuth == helpers.AwsIrsaAuthType {
if o.registrationOption.HubClusterArn != "" {
return awsIrsa.NewAWSOption(
secretOption,

View File

@@ -20,6 +20,7 @@ import (
ocmfeature "open-cluster-management.io/api/feature"
commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers"
commonoptions "open-cluster-management.io/ocm/pkg/common/options"
testingcommon "open-cluster-management.io/ocm/pkg/common/testing"
"open-cluster-management.io/ocm/pkg/features"
@@ -46,7 +47,7 @@ func TestValidate(t *testing.T) {
defaultCompletedOptions := NewSpokeAgentOptions()
defaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig"
awsCompletedOptionsHubArnMissing := *defaultCompletedOptions
awsCompletedOptionsHubArnMissing.RegistrationAuth = AwsIrsaAuthType
awsCompletedOptionsHubArnMissing.RegistrationAuth = commonhelpers.AwsIrsaAuthType
awsDefaultCompletedOptions := awsCompletedOptionsHubArnMissing
awsDefaultCompletedOptions.HubClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1"