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
23 changes: 22 additions & 1 deletion pkg/aws/actuator/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
awssdk "github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/iam"
iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types"
smithy "github.com/aws/smithy-go"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
Expand Down Expand Up @@ -653,6 +654,10 @@ func (a *AWSActuator) syncMint(ctx context.Context, cr *minterv1.CredentialsRequ
}
err = a.setUserPolicy(ctx, rootAWSClient, awsStatus.User, awsStatus.Policy, desiredUserPolicy)
if err != nil {
logger.WithFields(log.Fields{
"userName": awsStatus.User,
"policyName": awsStatus.Policy,
}).Error("failed to set user policy")
return err
}
logger.Info("successfully set user policy")
Expand Down Expand Up @@ -1186,7 +1191,15 @@ func (a *AWSActuator) setUserPolicy(ctx context.Context, awsClient ccaws.Client,
PolicyName: awssdk.String(policyName),
})
if err != nil {
return fmt.Errorf("unknown error setting user policy in AWS: %v", err)
if isAccessDenied(err) {
return fmt.Errorf("access denied setting IAM user policy (user: %s, policy: %s): "+
"this is typically caused by an AWS Service Control Policy (SCP) blocking iam:PutUserPolicy. "+
"To resolve, manually apply the following policy document to the IAM user, or adjust the SCP to allow iam:PutUserPolicy. "+
"Desired policy document: %s Original error: %v",
userName, policyName, userPolicy, err)
}
return fmt.Errorf("error setting user policy in AWS (user: %s, policy: %s): %v",
userName, policyName, err)
}

return nil
Expand Down Expand Up @@ -1337,6 +1350,14 @@ func (a *AWSActuator) loadClusterUUID(logger log.FieldLogger) (configv1.ClusterI
return clusterVer.Spec.ClusterID, nil
}

func isAccessDenied(err error) bool {
var apiErr smithy.APIError
if errors.As(err, &apiErr) {
return apiErr.ErrorCode() == "AccessDenied"
}
return false
}

func isAWSCredentials(providerSpec *runtime.RawExtension) (bool, error) {
unknown := runtime.Unknown{}
err := minterv1.Codec.DecodeProviderSpec(providerSpec, &unknown)
Expand Down
42 changes: 42 additions & 0 deletions pkg/aws/actuator/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

smithy "github.com/aws/smithy-go"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"

Expand Down Expand Up @@ -717,3 +719,43 @@ func testAuthentication(issuer string) *configv1.Authentication {
}
return conf
}

func TestIsAccessDenied(t *testing.T) {
tests := []struct {
name string
err error
expected bool
}{
{
name: "AccessDenied API error",
err: &smithy.GenericAPIError{Code: "AccessDenied", Message: "blocked by SCP"},
expected: true,
},
{
name: "wrapped AccessDenied",
err: fmt.Errorf("outer: %w", &smithy.GenericAPIError{Code: "AccessDenied", Message: "blocked"}),
expected: true,
},
{
name: "different error code",
err: &smithy.GenericAPIError{Code: "InvalidInput", Message: "bad input"},
expected: false,
},
{
name: "generic error",
err: fmt.Errorf("some other error"),
expected: false,
},
{
name: "nil error",
err: nil,
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isAccessDenied(tt.err)
assert.Equal(t, tt.expected, got)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,47 @@ func TestCredentialsRequestReconcile(t *testing.T) {
},
},
},
{
name: "SCP denied PutUserPolicy",
existing: []runtime.Object{
testOperatorConfig(""),
testInfrastructure(testInfraName),
createTestNamespace(testNamespace),
createTestNamespace(testSecretNamespace),
testCredentialsRequest(t),
testAWSCredsSecret("openshift-cloud-credential-operator", "cloud-credential-operator-iam-ro-creds", testReadAWSAccessKeyID, testReadAWSSecretAccessKey),
testClusterVersion(),
},
existingAdmin: []runtime.Object{
testAWSCredsSecret("kube-system", "aws-creds", testRootAWSAccessKeyID, testRootAWSSecretAccessKey),
},
mockRootAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient {
mockAWSClient := mockaws.NewMockClient(mockCtrl)
mockGetUser(mockAWSClient)
mockCreateUser(mockAWSClient)
mockTagUser(mockAWSClient)
mockPutUserPolicyAccessDenied(mockAWSClient)
return mockAWSClient
},
mockReadAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient {
mockAWSClient := mockaws.NewMockClient(mockCtrl)
mockGetUserNotFound(mockAWSClient)
mockGetUserPolicyMissing(mockAWSClient)
return mockAWSClient
},
validate: func(c client.Client, t *testing.T) {
cr := getCR(c)
assert.False(t, cr.Status.Provisioned)
},
expectErr: true,
expectedConditions: []ExpectedCondition{
{
conditionType: minterv1.CredentialsProvisionFailure,
reason: "CredentialsProvisionFailure",
status: corev1.ConditionTrue,
},
},
},
{
name: "cred deletion failure condition",
existing: []runtime.Object{
Expand Down Expand Up @@ -1962,6 +2003,16 @@ func mockPutUserPolicy(mockAWSClient *mockaws.MockClient) {
mockAWSClient.EXPECT().PutUserPolicy(gomock.Any(), gomock.Any()).Return(&iam.PutUserPolicyOutput{}, nil)
}

func mockPutUserPolicyAccessDenied(mockAWSClient *mockaws.MockClient) {
mockAWSClient.EXPECT().PutUserPolicy(gomock.Any(), gomock.Any()).Return(
nil,
&smithy.GenericAPIError{
Code: "AccessDenied",
Message: "User: arn:aws:iam::123456789:user/root is not authorized to perform: iam:PutUserPolicy with an explicit deny in a service control policy",
},
)
}

func mockGetUserPolicy(mockAWSClient *mockaws.MockClient, policyDoc string) {
policyDoc = url.QueryEscape(policyDoc)
mockAWSClient.EXPECT().GetUserPolicy(gomock.Any(), gomock.Any()).Return(&iam.GetUserPolicyOutput{
Expand Down