Skip to content

Commit

Permalink
Merge pull request #191 from openshift-cherrypick-robot/cherry-pick-1…
Browse files Browse the repository at this point in the history 
…61-to-release-4.4

[release-4.4] Bug 1838810: do not special-case handling read-only credentialsRequest
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Jun 3, 2020
2 parents 07e01d3 + 94e6e1e commit 37f5d3c
Show file tree
Hide file tree
Showing 2 changed files with 249 additions and 26 deletions.
32 changes: 7 additions & 25 deletions pkg/aws/actuator/actuator.go
View file
Expand Up @@ -123,14 +123,6 @@ func (a *AWSActuator) Exists(ctx context.Context, cr *minterv1.CredentialsReques
if isAWS, err := isAWSCredentials(cr.Spec.ProviderSpec); !isAWS {
return false, err
}
awsStatus, err := DecodeProviderStatus(a.Codec, cr)
if err != nil {
return false, err
}
if awsStatus.User == "" {
logger.Debug("username unset")
return false, nil
}

existingSecret := &corev1.Secret{}
err = a.Client.Get(context.TODO(), types.NamespacedName{Namespace: cr.Spec.SecretRef.Namespace, Name: cr.Spec.SecretRef.Name}, existingSecret)
Expand Down Expand Up @@ -767,23 +759,13 @@ func (a *AWSActuator) buildReadAWSClient(cr *minterv1.CredentialsRequest, region
var accessKeyID, secretAccessKey []byte
var err error

// Handle an edge case with management of our own RO creds using a credentials request.
// If we're operating on those credentials, just use the root creds.
if cr.Spec.SecretRef.Name == roAWSCredsSecret && cr.Spec.SecretRef.Namespace == roAWSCredsSecretNamespace {
log.Debug("operating our our RO creds, using root creds for all AWS client operations")
accessKeyID, secretAccessKey, err = utils.LoadCredsFromSecret(a.Client, rootAWSCredsSecretNamespace, rootAWSCredsSecret)
if err != nil {
return nil, err
}
} else {
// TODO: Running in a 4.0 cluster we expect this secret to exist. When we run in a Hive
// cluster, we need to load different secrets for each cluster.
accessKeyID, secretAccessKey, err = utils.LoadCredsFromSecret(a.Client, roAWSCredsSecretNamespace, roAWSCredsSecret)
if err != nil {
if errors.IsNotFound(err) {
logger.Warn("read-only creds not found, using root creds client")
return a.buildRootAWSClient(cr, region, infraName)
}
// TODO: Running in a 4.0 cluster we expect this secret to exist. When we run in a Hive
// cluster, we need to load different secrets for each cluster.
accessKeyID, secretAccessKey, err = utils.LoadCredsFromSecret(a.Client, roAWSCredsSecretNamespace, roAWSCredsSecret)
if err != nil {
if errors.IsNotFound(err) {
logger.Warn("read-only creds not found, using root creds client")
return a.buildRootAWSClient(cr, region, infraName)
}
}

Expand Down
243 changes: 242 additions & 1 deletion pkg/aws/actuator/actuator_test.go
View file
Expand Up @@ -17,11 +17,197 @@ limitations under the License.
package actuator

import (
"github.com/stretchr/testify/assert"
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"

"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/openshift/cloud-credential-operator/pkg/apis"
minterv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"
ccaws "github.com/openshift/cloud-credential-operator/pkg/aws"
mockaws "github.com/openshift/cloud-credential-operator/pkg/aws/mock"
)

const (
testROAccessKeyID = "TestROAccessKeyID"
testROSecretAccessKey = "TestROSecretAccessKey"

testRootAccessKeyID = "TestRootAccessKeyID"
testRootSecretAccessKey = "TestRootSecretAccessKey"
)

type awsClientBuilderRecorder struct {
accessKeyID []byte
secretAccessKey []byte

fakeAWSClient *mockaws.MockClient
fakeAWSClientError error
}

func (a *awsClientBuilderRecorder) ClientBuilder(accessKeyID, secretAccessKey []byte, region, infra string) (ccaws.Client, error) {
a.accessKeyID = accessKeyID
a.secretAccessKey = secretAccessKey

return a.fakeAWSClient, a.fakeAWSClientError
}

func TestCredentialsFetching(t *testing.T) {
apis.AddToScheme(scheme.Scheme)

codec, err := minterv1.NewCodec()
if err != nil {
t.Fatalf("failed to set up codec for tests: %v", err)
}

tests := []struct {
name string
existing []runtime.Object
credentialsRequest *minterv1.CredentialsRequest
expectedError bool
validate func(*testing.T, *awsClientBuilderRecorder)
clientBuilderRecorder func(*gomock.Controller) *awsClientBuilderRecorder
}{
{
name: "read only secret exists",
existing: []runtime.Object{
testReadOnlySecret(),
},
clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
r := &awsClientBuilderRecorder{}

awsClient := mockaws.NewMockClient(mockCtrl)
awsClient.EXPECT().GetUser(gomock.Any()).Return(nil, nil)

r.fakeAWSClient = awsClient

return r
},
validate: func(t *testing.T, clientRecorder *awsClientBuilderRecorder) {
assert.Equal(t, testROAccessKeyID, string(clientRecorder.accessKeyID))
assert.Equal(t, testROSecretAccessKey, string(clientRecorder.secretAccessKey))
},
},
{
name: "no read only secret",
existing: []runtime.Object{
testRootSecret(),
},
clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
r := &awsClientBuilderRecorder{}

awsClient := mockaws.NewMockClient(mockCtrl)
r.fakeAWSClient = awsClient

return r
},
validate: func(t *testing.T, clientRecorder *awsClientBuilderRecorder) {
assert.Equal(t, testRootAccessKeyID, string(clientRecorder.accessKeyID))
assert.Equal(t, testRootSecretAccessKey, string(clientRecorder.secretAccessKey))
},
},
{
name: "read only creds not ready",
existing: []runtime.Object{
testReadOnlySecret(),
testRootSecret(),
},
clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
r := &awsClientBuilderRecorder{}

awsClient := mockaws.NewMockClient(mockCtrl)
awsClient.EXPECT().GetUser(gomock.Any()).Return(nil, &testAWSError{
code: "InvalidClientTokenId",
})
r.fakeAWSClient = awsClient

return r
},
validate: func(t *testing.T, clientRecorder *awsClientBuilderRecorder) {
assert.Equal(t, testRootAccessKeyID, string(clientRecorder.accessKeyID))
assert.Equal(t, testRootSecretAccessKey, string(clientRecorder.secretAccessKey))
},
},
{
name: "error creating client",
expectedError: true,
existing: []runtime.Object{
testReadOnlySecret(),
},
clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
r := &awsClientBuilderRecorder{
fakeAWSClientError: fmt.Errorf("test error"),
}

return r
},
},
{
name: "bad credentials request",
expectedError: true,
existing: []runtime.Object{
testReadOnlySecret(),
},
credentialsRequest: func() *minterv1.CredentialsRequest {
cr := testCredentialsRequest()
cr.Status.ProviderStatus = &runtime.RawExtension{
Raw: []byte("garbage data"),
}

return cr
}(),
clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
r := &awsClientBuilderRecorder{}

fakeAWSClient := mockaws.NewMockClient(mockCtrl)
r.fakeAWSClient = fakeAWSClient

return r
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.credentialsRequest == nil {
test.credentialsRequest = testCredentialsRequest()
}

test.existing = append(test.existing, test.credentialsRequest)
fakeClient := fake.NewFakeClient(test.existing...)

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

clientRecorder := test.clientBuilderRecorder(mockCtrl)

a := &AWSActuator{
Client: fakeClient,
Codec: codec,
AWSClientBuilder: clientRecorder.ClientBuilder,
}

aClient, err := a.buildReadAWSClient(test.credentialsRequest, "testregion", "testinfra")

if test.expectedError {
assert.Error(t, err, "expected error for test case")
} else {
assert.NotNil(t, aClient)
if test.validate != nil {
test.validate(t, clientRecorder)
}
}
})
}
}
func TestGenerateUserName(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -82,3 +268,58 @@ func TestGenerateUserName(t *testing.T) {
})
}
}

func testReadOnlySecret() *corev1.Secret {
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: roAWSCredsSecret,
Namespace: roAWSCredsSecretNamespace,
},
Data: map[string][]byte{
"aws_access_key_id": []byte(testROAccessKeyID),
"aws_secret_access_key": []byte(testROSecretAccessKey),
},
}
}

func testRootSecret() *corev1.Secret {
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: rootAWSCredsSecret,
Namespace: rootAWSCredsSecretNamespace,
},
Data: map[string][]byte{
"aws_access_key_id": []byte(testRootAccessKeyID),
"aws_secret_access_key": []byte(testRootSecretAccessKey),
},
}
}

func testCredentialsRequest() *minterv1.CredentialsRequest {
return &minterv1.CredentialsRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "testcr",
Namespace: "testnamespace",
},
}
}

type testAWSError struct {
code string
}

func (a *testAWSError) Code() string {
return a.code
}

func (a *testAWSError) Message() string {
panic("not implemented")
}

func (a *testAWSError) OrigErr() error {
panic("not implemented")
}

func (a *testAWSError) Error() string {
panic("not implemented")
}

0 comments on commit 37f5d3c

Please sign in to comment.