-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ROX-12151: cloud accounts endpoint #517
Changes from all commits
66e9885
d95da4e
f083ec1
617a45a
a5c79f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package handlers | ||
|
||
import ( | ||
"net/http" | ||
|
||
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/presenters" | ||
"github.com/stackrox/acs-fleet-manager/pkg/auth" | ||
"github.com/stackrox/acs-fleet-manager/pkg/client/ocm" | ||
"github.com/stackrox/acs-fleet-manager/pkg/errors" | ||
"github.com/stackrox/acs-fleet-manager/pkg/handlers" | ||
) | ||
|
||
const rhacsMarketplaceQuotaID = "cluster|rhinfra|rhacs|marketplace" | ||
|
||
type cloudAccountsHandler struct { | ||
client ocm.AMSClient | ||
} | ||
|
||
// NewCloudAccountsHandler ... | ||
func NewCloudAccountsHandler(client ocm.AMSClient) *cloudAccountsHandler { | ||
return &cloudAccountsHandler{ | ||
client: client, | ||
} | ||
} | ||
|
||
// Get ... | ||
func (h *cloudAccountsHandler) Get(w http.ResponseWriter, r *http.Request) { | ||
cfg := &handlers.HandlerConfig{ | ||
Action: h.actionFunc(r), | ||
} | ||
handlers.HandleGet(w, r, cfg) | ||
} | ||
|
||
func (h *cloudAccountsHandler) actionFunc(r *http.Request) func() (i interface{}, serviceError *errors.ServiceError) { | ||
return func() (i interface{}, serviceError *errors.ServiceError) { | ||
ctx := r.Context() | ||
claims, err := auth.GetClaimsFromContext(ctx) | ||
if err != nil { | ||
return nil, errors.NewWithCause(errors.ErrorUnauthenticated, err, "user not authenticated") | ||
} | ||
orgID, err := claims.GetOrgID() | ||
if err != nil { | ||
return nil, errors.NewWithCause(errors.ErrorForbidden, err, "cannot make request without orgID claim") | ||
} | ||
|
||
cloudAccounts, err := h.client.GetCustomerCloudAccounts(orgID, []string{rhacsMarketplaceQuotaID}) | ||
if err != nil { | ||
return nil, errors.NewWithCause(errors.ErrorGeneral, err, "failed to fetch cloud accounts from AMS") | ||
} | ||
return presenters.PresentCloudAccounts(cloudAccounts), nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
package handlers | ||
|
||
import ( | ||
"context" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/stackrox/acs-fleet-manager/pkg/errors" | ||
|
||
"github.com/google/uuid" | ||
v1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" | ||
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/public" | ||
"github.com/stackrox/acs-fleet-manager/pkg/auth" | ||
"github.com/stackrox/acs-fleet-manager/pkg/client/ocm" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
const ( | ||
JwtKeyFile = "test/support/jwt_private_key.pem" | ||
JwtCAFile = "test/support/jwt_ca.pem" | ||
) | ||
|
||
func TestGetSuccess(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm yes that is impractical, I thought I saw it in the method list of the client, but you are right it's not. That would leave us with mocks on http level, which maybe are supported via the "mock modes" (https://github.com/stackrox/acs-fleet-manager/blob/ivan/ROX-12151-aws-accounts-endpoint/pkg/client/ocm/config.go#L12). But probably a lot of effort for a simple feature... |
||
testCloudAccount, err := v1.NewCloudAccount(). | ||
CloudAccountID("cloudAccountID"). | ||
CloudProviderID("cloudProviderID"). | ||
Build() | ||
assert.NoError(t, err) | ||
c := ocm.ClientMock{ | ||
GetCustomerCloudAccountsFunc: func(externalID string, quotaID []string) ([]*v1.CloudAccount, error) { | ||
return []*v1.CloudAccount{ | ||
testCloudAccount, | ||
}, nil | ||
}, | ||
} | ||
handler := NewCloudAccountsHandler(&c) | ||
|
||
authHelper, err := auth.NewAuthHelper(JwtKeyFile, JwtCAFile, "") | ||
assert.NoError(t, err) | ||
account, err := authHelper.NewAccount("username", "test-user", "", "org-id-0") | ||
assert.NoError(t, err) | ||
jwt, err := authHelper.CreateJWTWithClaims(account, nil) | ||
assert.NoError(t, err) | ||
authenticatedCtx := auth.SetTokenInContext(context.TODO(), jwt) | ||
r := &http.Request{} | ||
r = r.WithContext(authenticatedCtx) | ||
|
||
res, err := handler.actionFunc(r)() | ||
assert.Nil(t, err) | ||
cloudAccountsList, ok := res.(public.CloudAccountsList) | ||
assert.True(t, ok) | ||
|
||
assert.Len(t, cloudAccountsList.CloudAccounts, 1) | ||
assert.Equal(t, cloudAccountsList.CloudAccounts[0].CloudAccountId, testCloudAccount.CloudAccountID()) | ||
assert.Equal(t, cloudAccountsList.CloudAccounts[0].CloudProviderId, testCloudAccount.CloudProviderID()) | ||
} | ||
|
||
func TestGetNoOrgId(t *testing.T) { | ||
timesClientCalled := 0 | ||
c := ocm.ClientMock{ | ||
GetCustomerCloudAccountsFunc: func(externalID string, quotaID []string) ([]*v1.CloudAccount, error) { | ||
timesClientCalled++ | ||
return []*v1.CloudAccount{}, nil | ||
}, | ||
} | ||
handler := NewCloudAccountsHandler(&c) | ||
|
||
authHelper, err := auth.NewAuthHelper(JwtKeyFile, JwtCAFile, "") | ||
assert.NoError(t, err) | ||
builder := v1.NewAccount(). | ||
ID(uuid.New().String()). | ||
Username("username"). | ||
FirstName("Max"). | ||
LastName("M"). | ||
Email("[email protected]") | ||
account, err := builder.Build() | ||
assert.NoError(t, err) | ||
jwt, err := authHelper.CreateJWTWithClaims(account, nil) | ||
assert.NoError(t, err) | ||
authenticatedCtx := auth.SetTokenInContext(context.TODO(), jwt) | ||
r := &http.Request{} | ||
r = r.WithContext(authenticatedCtx) | ||
|
||
_, serviceErr := handler.actionFunc(r)() | ||
assert.Equal(t, serviceErr.Code, errors.ErrorForbidden) | ||
assert.Equal(t, 0, timesClientCalled) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package presenters | ||
|
||
import ( | ||
amsv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" | ||
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/public" | ||
) | ||
|
||
// PresentCloudAccounts ... | ||
func PresentCloudAccounts(cloudAccounts []*amsv1.CloudAccount) public.CloudAccountsList { | ||
transformedCloudAccounts := make([]public.CloudAccount, 0, len(cloudAccounts)) | ||
for _, cloudAccount := range cloudAccounts { | ||
transformedCloudAccounts = append(transformedCloudAccounts, public.CloudAccount{ | ||
CloudAccountId: cloudAccount.CloudAccountID(), | ||
CloudProviderId: cloudAccount.CloudProviderID(), | ||
}) | ||
} | ||
return public.CloudAccountsList{ | ||
CloudAccounts: transformedCloudAccounts, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint looks good to me. The only thing I'm wondering about is this
operationId
. @stehessel mentioned in his comment how to access theorganization.id
usingocm
command. IsoperationId
supposed to be that? Also, accessing that id using the ocm command is helpful, but I can't do that through the browser so I'll need to find another way to do that. If you or @stehessel might know how that's possible, let me know. I am searching through https://api.openshift.com/ to see if there might be some API endpoint that might give me that informationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
/api/accounts_mgmt/v1/organizations
the API i'm looking for? Looks like that gives me a list of organizations (there seems to be just one though). I see fields likeid
andexternal_id
which seem relevantThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sachaudh
organization.id
will be extracted byfleet-manager
from AMS usingorgId
provided in the Bearer token. The endpoint doesn't take any parameters.operationId
is swagger parameter that is used to uniquely identify the operation within the API spec:https://swagger.io/docs/specification/paths-and-operations/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then I think that's fine with me. Thanks for adding the endpoint @ivan-degtiarenko 👍🏼