From c0cd6f5106cddaa177aa193f7bbb1db448640683 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 29 Aug 2024 08:56:37 +0200 Subject: [PATCH 1/6] return cloud-credentials with empty attribute --- internal/jujuapi/cloud.go | 2 +- internal/jujuapi/cloud_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/internal/jujuapi/cloud.go b/internal/jujuapi/cloud.go index 1adb9b775..5723c9817 100644 --- a/internal/jujuapi/cloud.go +++ b/internal/jujuapi/cloud.go @@ -321,7 +321,7 @@ func getIdentityCredentials(ctx context.Context, user *openfga.User, j JIMM, arg } var err error content.Attributes, _, err = j.GetCloudCredentialAttributes(ctx, user, c, args.IncludeSecrets) - if err != nil { + if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { return nil, errors.E(err) } mas := make([]jujuparams.ModelAccess, len(c.Models)) diff --git a/internal/jujuapi/cloud_test.go b/internal/jujuapi/cloud_test.go index 58d4144e3..e2fa8a3b8 100644 --- a/internal/jujuapi/cloud_test.go +++ b/internal/jujuapi/cloud_test.go @@ -729,6 +729,33 @@ func (s *cloudSuite) TestCredentialContents(c *gc.C) { }}) } +func (s *cloudSuite) TestCredentialContentsWithEmptyAttributes(c *gc.C) { + conn := s.open(c, nil, "test") + defer conn.Close() + client := cloudapi.NewClient(conn) + credentialTag := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/test@canonical.com/cred3") + err := client.AddCredential( + credentialTag.String(), + cloud.NewCredential( + "certificate", + nil, + ), + ) + c.Assert(err, gc.Equals, nil) + creds, err := client.CredentialContents(jimmtest.TestCloudName, "cred3", false) + c.Assert(err, gc.Equals, nil) + c.Assert(creds, jc.DeepEquals, []jujuparams.CredentialContentResult{{ + Result: &jujuparams.ControllerCredentialInfo{ + Content: jujuparams.CredentialContent{ + Name: "cred3", + Cloud: jimmtest.TestCloudName, + AuthType: "certificate", + Attributes: nil, + }, + }, + }}) +} + func (s *cloudSuite) TestRemoveCloud(c *gc.C) { conn := s.open(c, nil, "test") defer conn.Close() From eb7eb51e6a1a42670a092abf260b8c7b44303991 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 29 Aug 2024 11:13:23 +0200 Subject: [PATCH 2/6] set empty map if attributes not found --- internal/jujuapi/cloud.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/jujuapi/cloud.go b/internal/jujuapi/cloud.go index 5723c9817..ba33787b7 100644 --- a/internal/jujuapi/cloud.go +++ b/internal/jujuapi/cloud.go @@ -321,8 +321,12 @@ func getIdentityCredentials(ctx context.Context, user *openfga.User, j JIMM, arg } var err error content.Attributes, _, err = j.GetCloudCredentialAttributes(ctx, user, c, args.IncludeSecrets) - if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { - return nil, errors.E(err) + if err != nil { + if errors.ErrorCode(err) == errors.CodeNotFound { + content.Attributes = map[string]string{} + } else { + return nil, errors.E(err) + } } mas := make([]jujuparams.ModelAccess, len(c.Models)) for i, m := range c.Models { From b66f8e65b4596bfec766e3693d551b69b56eb296 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 29 Aug 2024 16:25:17 +0200 Subject: [PATCH 3/6] change application logic to not return error on empty attributes --- internal/jimm/cloudcredential.go | 3 --- internal/jujuapi/cloud.go | 6 +----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/internal/jimm/cloudcredential.go b/internal/jimm/cloudcredential.go index 9e710a982..037ebcd87 100644 --- a/internal/jimm/cloudcredential.go +++ b/internal/jimm/cloudcredential.go @@ -377,8 +377,5 @@ func (j *JIMM) getCloudCredentialAttributes(ctx context.Context, cred *dbmodel.C if err != nil { return nil, errors.E(op, err) } - if len(attr) == 0 && cred.AuthType != "empty" { - return nil, errors.E(op, errors.CodeNotFound, "cloud-credential attributes not found") - } return attr, nil } diff --git a/internal/jujuapi/cloud.go b/internal/jujuapi/cloud.go index ba33787b7..1adb9b775 100644 --- a/internal/jujuapi/cloud.go +++ b/internal/jujuapi/cloud.go @@ -322,11 +322,7 @@ func getIdentityCredentials(ctx context.Context, user *openfga.User, j JIMM, arg var err error content.Attributes, _, err = j.GetCloudCredentialAttributes(ctx, user, c, args.IncludeSecrets) if err != nil { - if errors.ErrorCode(err) == errors.CodeNotFound { - content.Attributes = map[string]string{} - } else { - return nil, errors.E(err) - } + return nil, errors.E(err) } mas := make([]jujuparams.ModelAccess, len(c.Models)) for i, m := range c.Models { From f594b5f277385c160295e4bd5c0d327ef64dcd89 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 29 Aug 2024 17:28:57 +0200 Subject: [PATCH 4/6] add app layer test --- internal/jimm/cloudcredential_test.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 775b48d23..f711a7ed3 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -5,6 +5,7 @@ package jimm_test import ( "context" "database/sql" + "fmt" "sync" "testing" "time" @@ -1538,6 +1539,10 @@ cloud-credentials: client-id: 1234 private-key: super-secret project-id: 5678 +- name: cred-2 + cloud: test-cloud + owner: bob@canonical.com + auth-type: certificate users: - username: alice@canonical.com controller-access: superuser @@ -1549,6 +1554,7 @@ var getCloudCredentialAttributesTests = []struct { username string hidden bool jimmAdmin bool + cred string expectAttributes map[string]string expectRedacted []string expectError string @@ -1557,16 +1563,25 @@ var getCloudCredentialAttributesTests = []struct { name: "OwnerNoHidden", username: "bob@canonical.com", jimmAdmin: true, + cred: "cred-1", expectAttributes: map[string]string{ "client-email": "bob@example.com", "client-id": "1234", "project-id": "5678", }, expectRedacted: []string{"private-key"}, +}, { + name: "OwnerNoAttributes", + username: "bob@canonical.com", + jimmAdmin: true, + cred: "cred-2", + expectAttributes: nil, + expectRedacted: nil, }, { name: "OwnerWithHidden", username: "bob@canonical.com", hidden: true, + cred: "cred-1", expectAttributes: map[string]string{ "client-email": "bob@example.com", "client-id": "1234", @@ -1577,6 +1592,7 @@ var getCloudCredentialAttributesTests = []struct { name: "SuperUserNoHidden", username: "alice@canonical.com", jimmAdmin: true, + cred: "cred-1", expectAttributes: map[string]string{ "client-email": "bob@example.com", "client-id": "1234", @@ -1588,11 +1604,13 @@ var getCloudCredentialAttributesTests = []struct { username: "alice@canonical.com", hidden: true, jimmAdmin: true, + cred: "cred-1", expectError: `unauthorized`, expectErrorCode: errors.CodeUnauthorized, }, { name: "OtherUserUnauthorized", username: "charlie@canonical.com", + cred: "cred-1", expectError: `unauthorized`, expectErrorCode: errors.CodeUnauthorized, }} @@ -1623,7 +1641,8 @@ func TestGetCloudCredentialAttributes(t *testing.T) { env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client) u := env.User("bob@canonical.com").DBObject(c, j.Database) userBob := openfga.NewUser(&u, client) - cred, err := j.GetCloudCredential(ctx, userBob, names.NewCloudCredentialTag("test-cloud/bob@canonical.com/cred-1")) + credTag := fmt.Sprintf("test-cloud/bob@canonical.com/%s", test.cred) + cred, err := j.GetCloudCredential(ctx, userBob, names.NewCloudCredentialTag(credTag)) c.Assert(err, qt.IsNil) u = env.User(test.username).DBObject(c, j.Database) From 896f58d84b9847d8b62045fa2a6b8aa9b732fddb Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 29 Aug 2024 17:34:37 +0200 Subject: [PATCH 5/6] return empty map rather than nil --- internal/jimm/cloudcredential.go | 3 +++ internal/jimm/cloudcredential_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/jimm/cloudcredential.go b/internal/jimm/cloudcredential.go index 037ebcd87..5691a2c06 100644 --- a/internal/jimm/cloudcredential.go +++ b/internal/jimm/cloudcredential.go @@ -342,6 +342,9 @@ func (j *JIMM) GetCloudCredentialAttributes(ctx context.Context, user *openfga.U err = errors.E(op, err) return } + if len(attrs) == 0 { + return map[string]string{}, nil, nil + } if hidden { return diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index f711a7ed3..66a9a8c67 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -1575,7 +1575,7 @@ var getCloudCredentialAttributesTests = []struct { username: "bob@canonical.com", jimmAdmin: true, cred: "cred-2", - expectAttributes: nil, + expectAttributes: map[string]string{}, expectRedacted: nil, }, { name: "OwnerWithHidden", From fd97f0873126d953fc4f2378e584bb50b483830c Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 30 Aug 2024 09:04:11 +0200 Subject: [PATCH 6/6] fix test --- internal/jimm/cloudcredential_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 66a9a8c67..4e4151e4d 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -1733,7 +1733,7 @@ func TestCloudCredentialAttributeStore(t *testing.T) { // Update to an "empty" credential args.Credential.AuthType = "empty" - args.Credential.Attributes = nil + args.Credential.Attributes = map[string]string{} _, err = j.UpdateCloudCredential(ctx, user, args) c.Assert(err, qt.IsNil)