From fa0bda6f7b0e8a19c8bc12ccc6d5b704c8eef0e0 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 5 Apr 2023 18:27:08 +0100 Subject: [PATCH 1/9] Integrations: web api and tclt This PR adds end user interface to manage integrations: `tctl` ``` $ tctl get integrations --config teleport.yaml --format text Name Type Spec ----------- -------- ---------------------------------------------- myawsint aws-oidc RoleARN=arn:aws:iam::123456789012:role/DevTeam mynewawsint aws-oidc RoleARN=arn:aws:iam::123456789012:role/OpsTeam ``` HTTP API ``` $ curl 'https://127.0.0.1.nip.io:3080/v1/webapi/sites/lenix/integrations' { "items": [ { "name": "myawsint", "subKind": "aws-oidc", "awsOIDC": { "roleARN": "arn:aws:iam::123456789012:role/DevTeam" } }, { "name": "mynewawsint", "subKind": "aws-oidc", "awsOIDC": { "roleARN": "arn:aws:iam::123456789012:role/OpsTeam" } } ], "nextKey": "" } ``` --- integration/helpers/web.go | 16 +- integration/integrations/integration_test.go | 232 +++++++++++++++++++ lib/services/resource.go | 2 + lib/web/apiserver.go | 7 + lib/web/integrations.go | 185 +++++++++++++++ lib/web/ui/integration.go | 104 +++++++++ tool/tctl/common/collection.go | 30 +++ tool/tctl/common/resource_command.go | 124 ++++++++-- tool/tctl/common/resource_command_test.go | 126 ++++++++++ 9 files changed, 798 insertions(+), 28 deletions(-) create mode 100644 integration/integrations/integration_test.go create mode 100644 lib/web/integrations.go create mode 100644 lib/web/ui/integration.go diff --git a/integration/helpers/web.go b/integration/helpers/web.go index 41d1ff5dda013..17449991c108a 100644 --- a/integration/helpers/web.go +++ b/integration/helpers/web.go @@ -19,6 +19,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "io" "net/http" "net/url" "strings" @@ -104,8 +105,13 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac require.NoError(t, err) defer resp.Body.Close() + bs, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, http.StatusOK, resp.StatusCode, string(bs)) + var clusters []ui.Cluster - require.NoError(t, json.NewDecoder(resp.Body).Decode(&clusters)) + require.NoError(t, json.Unmarshal(bs, &clusters), string(bs)) require.NotEmpty(t, clusters) webClient.clusterName = clusters[0].Name @@ -116,11 +122,11 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac // The endpoint must not contain the host neither the base path ('/v1/webapi/'). // Returns the http.Response. func (w *WebClientPack) DoRequest(method, endpoint string, payload any) (*http.Response, error) { + endpoint = fmt.Sprintf("https://%s/v1/webapi/%s", w.host, endpoint) endpoint = strings.ReplaceAll(endpoint, "$site", w.clusterName) - u := url.URL{ - Scheme: "https", - Host: w.host, - Path: fmt.Sprintf("/v1/webapi/%s", endpoint), + u, err := url.Parse(endpoint) + if err != nil { + return nil, trace.Wrap(err) } bs, err := json.Marshal(payload) diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go new file mode 100644 index 0000000000000..ddfed549a8d2e --- /dev/null +++ b/integration/integrations/integration_test.go @@ -0,0 +1,232 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integrations + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/integration/helpers" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/web/ui" +) + +// TestIntegraitionCRUD starts a Teleport cluster and using its Proxy Web server, +// tests the CRUD operations over the Integration resource. +func TestIntegrationCRUD(t *testing.T) { + ctx := context.Background() + + // Start Teleport Auth and Proxy services + authProcess, proxyProcess, _ := helpers.MakeTestServers(t) + authServer := authProcess.GetAuthServer() + proxyAddr, err := proxyProcess.ProxyWebAddr() + require.NoError(t, err) + + roleWithFullAccess, err := types.NewRole("fullaccess", types.RoleSpecV6{ + Allow: types.RoleConditions{ + Namespaces: []string{apidefaults.Namespace}, + Rules: []types.Rule{ + types.NewRule(types.KindIntegration, services.RW()), + }, + }, + }) + require.NoError(t, err) + require.NoError(t, authServer.UpsertRole(ctx, roleWithFullAccess)) + + integrationsEndpoint := strings.Join([]string{"sites", "$site", "integrations"}, "/") + + // Set up User + username := "fullaccess" + user, err := types.NewUser(username) + require.NoError(t, err) + + user.AddRole(roleWithFullAccess.GetName()) + require.NoError(t, authServer.UpsertUser(user)) + + userPassword := uuid.NewString() + require.NoError(t, authServer.UpsertPassword(username, []byte(userPassword))) + + webPack := helpers.LoginWebClient(t, proxyAddr.String(), username, userPassword) + + // List integrations should return empty + resp, err := webPack.DoRequest(http.MethodGet, integrationsEndpoint, nil) + require.NoError(t, err) + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + listResp := ui.IntegrationsListResponse{} + require.NoError(t, json.Unmarshal(respBody, &listResp)) + + require.Empty(t, listResp.Items) + + // Create Integration + createIntegrationReq := ui.Integration{ + Name: "MyAWSAccount", + SubKind: types.IntegrationSubKindAWSOIDC, + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/DevTeam", + }, + } + + resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + // Get One Integration by name + resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + integrationResp := ui.Integration{} + require.NoError(t, json.Unmarshal(respBody, &integrationResp)) + + require.Equal(t, ui.Integration{ + Name: "MyAWSAccount", + SubKind: types.IntegrationSubKindAWSOIDC, + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/DevTeam", + }, + }, integrationResp, string(respBody)) + + // Update the integration to another RoleARN + resp, err = webPack.DoRequest(http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", + }, + }) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + integrationResp = ui.Integration{} + require.NoError(t, json.Unmarshal(respBody, &integrationResp)) + + require.Equal(t, ui.Integration{ + Name: "MyAWSAccount", + SubKind: types.IntegrationSubKindAWSOIDC, + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", + }, + }, integrationResp, string(respBody)) + + // Delete resource + resp, err = webPack.DoRequest(http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + // Add multiple integrations to test pagination + // Tests two full pages + 1 item to prevent off by one errors. + pageSize := 10 + totalItems := pageSize*2 + 1 + for i := 0; i < totalItems; i++ { + createIntegrationReq := ui.Integration{ + Name: fmt.Sprintf("AWSIntegration%d", i), + SubKind: types.IntegrationSubKindAWSOIDC, + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/DevTeam", + }, + } + + resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + } + + // List integrations should return a full page + resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10", nil) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + listResp = ui.IntegrationsListResponse{} + require.NoError(t, json.Unmarshal(respBody, &listResp)) + + require.Len(t, listResp.Items, pageSize) + + // Requesting the 2nd page should return a full page + resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + listResp = ui.IntegrationsListResponse{} + require.NoError(t, json.Unmarshal(respBody, &listResp)) + + require.Len(t, listResp.Items, pageSize) + + // Requesting the 3rd page should return a single item and empty StartKey + resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.NoError(t, err) + + respBody, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + + listResp = ui.IntegrationsListResponse{} + require.NoError(t, json.Unmarshal(respBody, &listResp)) + + require.Len(t, listResp.Items, 1) + require.Empty(t, listResp.NextKey) +} diff --git a/lib/services/resource.go b/lib/services/resource.go index 4350afdada514..626845504f404 100644 --- a/lib/services/resource.go +++ b/lib/services/resource.go @@ -195,6 +195,8 @@ func ParseShortcut(in string) (string, error) { return types.KindOktaAssignment, nil case types.KindClusterMaintenanceConfig, "cmc": return types.KindClusterMaintenanceConfig, nil + case types.KindIntegration, types.KindIntegration + "s": + return types.KindIntegration, nil } return "", trace.BadParameter("unsupported resource: %q - resources should be expressed as 'type/name', for example 'connector/github'", in) } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index ca78970b1e18d..b5557dd9a391b 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -703,6 +703,13 @@ func (h *Handler) bindDefaultEndpoints() { // Diagnose a Connection h.POST("/webapi/sites/:site/diagnostics/connections", h.WithClusterAuth(h.diagnoseConnection)) + // Integrations CRUD + h.GET("/webapi/sites/:site/integrations", h.WithClusterAuth(h.integrationsList)) + h.POST("/webapi/sites/:site/integrations", h.WithClusterAuth(h.integrationsCreate)) + h.GET("/webapi/sites/:site/integrations/:name", h.WithClusterAuth(h.integrationsGet)) + h.PUT("/webapi/sites/:site/integrations/:name", h.WithClusterAuth(h.integrationsUpdate)) + h.DELETE("/webapi/sites/:site/integrations/:name", h.WithClusterAuth(h.integrationsDelete)) + // Connection upgrades. h.GET("/webapi/connectionupgrade", httplib.MakeHandler(h.connectionUpgrade)) diff --git a/lib/web/integrations.go b/lib/web/integrations.go new file mode 100644 index 0000000000000..ecfd866a95b6a --- /dev/null +++ b/lib/web/integrations.go @@ -0,0 +1,185 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package web + +import ( + "net/http" + + "github.com/gravitational/trace" + "github.com/julienschmidt/httprouter" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/httplib" + "github.com/gravitational/teleport/lib/reversetunnel" + "github.com/gravitational/teleport/lib/web/ui" +) + +// integrationsCreate creates an Integration +func (h *Handler) integrationsCreate(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnel.RemoteSite) (interface{}, error) { + var req *ui.Integration + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + if err := req.CheckAndSetDefaults(); err != nil { + return nil, trace.Wrap(err) + } + + var ig *types.IntegrationV1 + var err error + + switch req.SubKind { + case types.IntegrationSubKindAWSOIDC: + ig, err = types.NewIntegrationAWSOIDC( + types.Metadata{Name: req.Name}, + &types.AWSOIDCIntegrationSpecV1{ + RoleARN: req.AWSOIDC.RoleARN, + }, + ) + + if err != nil { + return nil, trace.Wrap(err) + } + + default: + return nil, trace.BadParameter("subkind %q is not supported", req.SubKind) + } + + clt, err := sctx.GetUserClient(r.Context(), site) + if err != nil { + return nil, trace.Wrap(err) + } + + storedIntegration, err := clt.CreateIntegration(r.Context(), ig) + if err != nil { + if trace.IsAlreadyExists(err) { + return nil, trace.AlreadyExists("failed to create Integration (%q already exists), please use another name", req.Name) + } + return nil, trace.Wrap(err) + } + + return ui.MakeIntegration(storedIntegration), nil +} + +// integrationsUpdate updates the Integration based on its name +func (h *Handler) integrationsUpdate(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnel.RemoteSite) (interface{}, error) { + integrationName := p.ByName("name") + if integrationName == "" { + return nil, trace.BadParameter("an integration name is required") + } + + var req *ui.UpdateIntegrationRequest + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + if err := req.CheckAndSetDefaults(); err != nil { + return nil, trace.Wrap(err) + } + + clt, err := sctx.GetUserClient(r.Context(), site) + if err != nil { + return nil, trace.Wrap(err) + } + + integration, err := clt.GetIntegration(r.Context(), integrationName) + if err != nil { + return nil, trace.Wrap(err) + } + + if req.AWSOIDC != nil { + if integration.GetSubKind() != types.IntegrationSubKindAWSOIDC { + return nil, trace.BadParameter("cannot update %q fields for a %q integration", types.IntegrationSubKindAWSOIDC, integration.GetSubKind()) + } + + spec := integration.GetAWSOIDCIntegrationSpec() + spec.RoleARN = req.AWSOIDC.RoleARN + integration.SetAWSOIDCIntegrationSpec(spec) + } + + if _, err := clt.UpdateIntegration(r.Context(), integration); err != nil { + return nil, trace.Wrap(err) + } + + return ui.MakeIntegration(integration), nil +} + +// integrationsDelete removes an Integration based on its name +func (h *Handler) integrationsDelete(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnel.RemoteSite) (interface{}, error) { + integrationName := p.ByName("name") + if integrationName == "" { + return nil, trace.BadParameter("an integration name is required") + } + + clt, err := sctx.GetUserClient(r.Context(), site) + if err != nil { + return nil, trace.Wrap(err) + } + + if err := clt.DeleteIntegration(r.Context(), integrationName); err != nil { + return nil, trace.Wrap(err) + } + + return nil, nil +} + +// integrationsGet returns an Integration based on its name +func (h *Handler) integrationsGet(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnel.RemoteSite) (interface{}, error) { + integrationName := p.ByName("name") + if integrationName == "" { + return nil, trace.BadParameter("an integration name is required") + } + + clt, err := sctx.GetUserClient(r.Context(), site) + if err != nil { + return nil, trace.Wrap(err) + } + + ig, err := clt.GetIntegration(r.Context(), integrationName) + if err != nil { + return nil, trace.Wrap(err) + } + + return ui.MakeIntegration(ig), nil +} + +// integrationsList returns a page of Integrations +func (h *Handler) integrationsList(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnel.RemoteSite) (interface{}, error) { + clt, err := sctx.GetUserClient(r.Context(), site) + if err != nil { + return nil, trace.Wrap(err) + } + + values := r.URL.Query() + limit, err := queryLimitAsInt32(values, "limit", defaults.MaxIterationLimit) + if err != nil { + return nil, trace.Wrap(err) + } + + startKey := values.Get("startKey") + + igs, nextKey, err := clt.ListIntegrations(r.Context(), int(limit), startKey) + if err != nil { + return nil, trace.Wrap(err) + } + + return ui.IntegrationsListResponse{ + Items: ui.MakeIntegrations(igs), + NextKey: nextKey, + }, nil +} diff --git a/lib/web/ui/integration.go b/lib/web/ui/integration.go new file mode 100644 index 0000000000000..1ed22b427f0e9 --- /dev/null +++ b/lib/web/ui/integration.go @@ -0,0 +1,104 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ui + +import ( + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/api/types" +) + +// IntegrationAWSOIDCSpec contain the specific fields for the `aws-oidc` subkind integration. +type IntegrationAWSOIDCSpec struct { + // RoleARN is the role associated with the integration when SubKind is `aws-oidc` + RoleARN string `json:"roleArn,omitempty"` +} + +// Integration describes the base Integration fields (can be read or wri) Integration +type Integration struct { + // Name is the Integration name. + Name string `json:"name"` + // SubKind is the Integration SubKind. + SubKind string `json:"subKind"` + // AWSOIDC contains the fields for `aws-oidc` subkind integration. + AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc"` +} + +// CheckAndSetDefaults for the create request. +// Name and SubKind is required. +func (r *Integration) CheckAndSetDefaults() error { + if r.Name == "" { + return trace.BadParameter("missing integration name") + } + + if r.SubKind == "" { + return trace.BadParameter("missing subKind") + } + + if r.AWSOIDC != nil && r.AWSOIDC.RoleARN == "" { + return trace.BadParameter("missing awsoidc.roleArn field") + } + + return nil +} + +// UpdateIntegrationRequest is a request to update an Integration +type UpdateIntegrationRequest struct { + // AWSOIDC contains the fields for `aws-oidc` subkind integration. + AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc"` +} + +// CheckAndSetDefaults checks if the provided values are valid. +func (r *UpdateIntegrationRequest) CheckAndSetDefaults() error { + if r.AWSOIDC != nil && r.AWSOIDC.RoleARN == "" { + return trace.BadParameter("missing awsoidc.roleArn field") + } + + return nil +} + +// IntegrationsListResponse contains a list of Integrations. +// In case of exceeding the pagination limit (either via query param `limit` or the default 1000) +// a `nextToken` is provided and should be used to obtain the next page (as a query param `startKey`) +type IntegrationsListResponse struct { + // Items is a list of resources retrieved. + Items interface{} `json:"items"` + // NextKey is the position to resume listing events. + NextKey string `json:"nextKey"` +} + +// MakeIntegrations creates a UI list of Integrations. +func MakeIntegrations(igs []types.Integration) []Integration { + uiList := make([]Integration, 0, len(igs)) + + for _, ig := range igs { + uiList = append(uiList, MakeIntegration(ig)) + } + + return uiList +} + +// MakeIntegration creates a UI Integration representation. +func MakeIntegration(ig types.Integration) Integration { + return Integration{ + Name: ig.GetName(), + SubKind: ig.GetSubKind(), + AWSOIDC: &IntegrationAWSOIDCSpec{ + RoleARN: ig.GetAWSOIDCIntegrationSpec().RoleARN, + }, + } +} diff --git a/tool/tctl/common/collection.go b/tool/tctl/common/collection.go index 0cd69702b9e53..0fd12bd12a72e 100644 --- a/tool/tctl/common/collection.go +++ b/tool/tctl/common/collection.go @@ -982,6 +982,36 @@ func (c *installerCollection) writeText(w io.Writer) error { return nil } +type integrationCollection struct { + integrations []types.Integration +} + +func (c *integrationCollection) resources() (r []types.Resource) { + for _, ig := range c.integrations { + r = append(r, ig) + } + return r +} +func (c *integrationCollection) writeText(w io.Writer) error { + sort.Sort(types.Integrations(c.integrations)) + var rows [][]string + for _, ig := range c.integrations { + specProps := []string{} + switch ig.GetSubKind() { + case types.IntegrationSubKindAWSOIDC: + specProps = append(specProps, fmt.Sprintf("RoleARN=%s", ig.GetAWSOIDCIntegrationSpec().RoleARN)) + } + + rows = append(rows, []string{ + ig.GetName(), ig.GetSubKind(), strings.Join(specProps, ","), + }) + } + headers := []string{"Name", "SubKind", "Spec"} + t := asciitable.MakeTable(headers, rows...) + _, err := t.AsBuffer().WriteTo(w) + return trace.Wrap(err) +} + type databaseServiceCollection struct { databaseServices []types.DatabaseService } diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index cbecca77e83f9..650b49a98a8d6 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -101,30 +101,31 @@ Same as above, but using JSON output: // Initialize allows ResourceCommand to plug itself into the CLI parser func (rc *ResourceCommand) Initialize(app *kingpin.Application, config *servicecfg.Config) { rc.CreateHandlers = map[ResourceKind]ResourceCreateHandler{ - types.KindUser: rc.createUser, - types.KindRole: rc.createRole, - types.KindTrustedCluster: rc.createTrustedCluster, - types.KindGithubConnector: rc.createGithubConnector, - types.KindCertAuthority: rc.createCertAuthority, - types.KindClusterAuthPreference: rc.createAuthPreference, - types.KindClusterNetworkingConfig: rc.createClusterNetworkingConfig, + types.KindUser: rc.createUser, + types.KindRole: rc.createRole, + types.KindTrustedCluster: rc.createTrustedCluster, + types.KindGithubConnector: rc.createGithubConnector, + types.KindCertAuthority: rc.createCertAuthority, + types.KindClusterAuthPreference: rc.createAuthPreference, + types.KindClusterNetworkingConfig: rc.createClusterNetworkingConfig, types.KindClusterMaintenanceConfig: rc.createClusterMaintenanceConfig, - types.KindSessionRecordingConfig: rc.createSessionRecordingConfig, - types.KindUIConfig: rc.createUIConfig, - types.KindLock: rc.createLock, - types.KindNetworkRestrictions: rc.createNetworkRestrictions, - types.KindApp: rc.createApp, - types.KindDatabase: rc.createDatabase, - types.KindKubernetesCluster: rc.createKubeCluster, - types.KindToken: rc.createToken, - types.KindInstaller: rc.createInstaller, - types.KindNode: rc.createNode, - types.KindOIDCConnector: rc.createOIDCConnector, - types.KindSAMLConnector: rc.createSAMLConnector, - types.KindLoginRule: rc.createLoginRule, - types.KindSAMLIdPServiceProvider: rc.createSAMLIdPServiceProvider, - types.KindDevice: rc.createDevice, - types.KindOktaImportRule: rc.createOktaImportRule, + types.KindSessionRecordingConfig: rc.createSessionRecordingConfig, + types.KindUIConfig: rc.createUIConfig, + types.KindLock: rc.createLock, + types.KindNetworkRestrictions: rc.createNetworkRestrictions, + types.KindApp: rc.createApp, + types.KindDatabase: rc.createDatabase, + types.KindKubernetesCluster: rc.createKubeCluster, + types.KindToken: rc.createToken, + types.KindInstaller: rc.createInstaller, + types.KindNode: rc.createNode, + types.KindOIDCConnector: rc.createOIDCConnector, + types.KindSAMLConnector: rc.createSAMLConnector, + types.KindLoginRule: rc.createLoginRule, + types.KindSAMLIdPServiceProvider: rc.createSAMLIdPServiceProvider, + types.KindDevice: rc.createDevice, + types.KindOktaImportRule: rc.createOktaImportRule, + types.KindIntegration: rc.createIntegration, } rc.config = config @@ -872,6 +873,51 @@ func (rc *ResourceCommand) createOktaImportRule(ctx context.Context, client auth return nil } +func (rc *ResourceCommand) createIntegration(ctx context.Context, client auth.ClientI, raw services.UnknownResource) error { + integration, err := services.UnmarshalIntegration(raw.Raw) + if err != nil { + return trace.Wrap(err) + } + + existingIntegration, err := client.GetIntegration(ctx, integration.GetName()) + if err != nil && !trace.IsNotFound(err) { + return trace.Wrap(err) + } + exists := (err == nil) + + if exists { + if !rc.force { + return trace.AlreadyExists("Integration %q already exists", integration.GetName()) + } + + switch integration.GetSubKind() { + case types.IntegrationSubKindAWSOIDC: + existingIntegration.SetAWSOIDCIntegrationSpec(integration.GetAWSOIDCIntegrationSpec()) + + default: + return trace.BadParameter("subkind %q is not supported", integration.GetSubKind()) + } + + if _, err := client.UpdateIntegration(ctx, existingIntegration); err != nil { + return trace.Wrap(err) + } + fmt.Printf("Integration %q has been updated\n", integration.GetName()) + return nil + } + + igV1, ok := integration.(*types.IntegrationV1) + if !ok { + return trace.BadParameter("unexpected Integration type %T", integration) + } + + if _, err := client.CreateIntegration(ctx, igV1); err != nil { + return trace.Wrap(err) + } + fmt.Printf("Integration %q has been created\n", integration.GetName()) + + return nil +} + // Delete deletes resource by name func (rc *ResourceCommand) Delete(ctx context.Context, client auth.ClientI) (err error) { singletonResources := []string{ @@ -1126,6 +1172,12 @@ func (rc *ResourceCommand) Delete(ctx context.Context, client auth.ClientI) (err } fmt.Printf("Device %q removed\n", rc.ref.Name) + case types.KindIntegration: + if err := client.DeleteIntegration(ctx, rc.ref.Name); err != nil { + return trace.Wrap(err) + } + fmt.Printf("Integration %q removed\n", rc.ref.Name) + case types.KindAppServer: appServers, err := client.GetApplicationServers(ctx, rc.namespace) if err != nil { @@ -1883,6 +1935,32 @@ func (rc *ResourceCommand) getCollection(ctx context.Context, client auth.Client } } return &userGroupCollection{userGroups: resources}, nil + + case types.KindIntegration: + if rc.ref.Name != "" { + ig, err := client.GetIntegration(ctx, rc.ref.Name) + if err != nil { + return nil, trace.Wrap(err) + } + return &integrationCollection{integrations: []types.Integration{ig}}, nil + } + + var resources []types.Integration + var igs []types.Integration + var err error + var nextKey string + for { + igs, nextKey, err = client.ListIntegrations(ctx, 0, nextKey) + if err != nil { + return nil, trace.Wrap(err) + } + resources = append(resources, igs...) + if nextKey == "" { + break + } + } + + return &integrationCollection{integrations: resources}, nil } return nil, trace.BadParameter("getting %q is not supported", rc.ref.String()) } diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index 19686274ef668..7339065cc44f2 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "time" @@ -320,6 +321,120 @@ func TestDatabaseServiceResource(t *testing.T) { }) } +// TestIntegrationResource tests tctl integration commands. +func TestIntegrationResource(t *testing.T) { + dynAddr := newDynamicServiceAddr(t) + + ctx := context.Background() + fileConfig := &config.FileConfig{ + Global: config.Global{ + DataDir: t.TempDir(), + }, + Proxy: config.Proxy{ + Service: config.Service{ + EnabledFlag: "true", + }, + WebAddr: dynAddr.webAddr, + TunAddr: dynAddr.tunnelAddr, + }, + Auth: config.Auth{ + Service: config.Service{ + EnabledFlag: "true", + ListenAddress: dynAddr.authAddr, + }, + }, + } + + auth := makeAndRunTestAuthServer(t, withFileConfig(fileConfig), withFileDescriptors(dynAddr.descriptors)) + + t.Run("get", func(t *testing.T) { + + var out []types.IntegrationV1 + + // Add a lot of Integrations to test pagination + ig1, err := types.NewIntegrationAWSOIDC( + types.Metadata{Name: uuid.NewString()}, + &types.AWSOIDCIntegrationSpecV1{ + RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", + }, + ) + require.NoError(t, err) + + randomIntegrationName := "" + totalIntegrations := apidefaults.DefaultChunkSize*2 + 20 // testing partial pages + for i := 0; i < totalIntegrations; i++ { + ig1.SetName(uuid.NewString()) + if i == apidefaults.DefaultChunkSize { // A "random" integration name + randomIntegrationName = ig1.GetName() + } + _, err = auth.GetAuthServer().CreateIntegration(ctx, ig1) + require.NoError(t, err) + } + + t.Run("test pagination of integrations ", func(t *testing.T) { + buff, err := runResourceCommand(t, fileConfig, []string{"get", types.KindIntegration, "--format=json"}) + require.NoError(t, err) + mustDecodeJSON(t, buff, &out) + require.Len(t, out, totalIntegrations) + }) + + igName := fmt.Sprintf("%v/%v", types.KindIntegration, randomIntegrationName) + + t.Run("get specific integration", func(t *testing.T) { + buff, err := runResourceCommand(t, fileConfig, []string{"get", igName, "--format=json"}) + require.NoError(t, err) + mustDecodeJSON(t, buff, &out) + require.Len(t, out, 1) + require.Equal(t, randomIntegrationName, out[0].GetName()) + }) + + t.Run("get unknown integration", func(t *testing.T) { + unknownIntegration := fmt.Sprintf("%v/%v", types.KindIntegration, "unknown") + _, err := runResourceCommand(t, fileConfig, []string{"get", unknownIntegration, "--format=json"}) + require.True(t, trace.IsNotFound(err), "expected a NotFound error, got %v", err) + }) + + t.Run("get specific integration with human output", func(t *testing.T) { + buff, err := runResourceCommand(t, fileConfig, []string{"get", igName, "--format=text"}) + require.NoError(t, err) + outputString := buff.String() + require.Contains(t, outputString, "RoleARN=arn:aws:iam::123456789012:role/OpsTeam") + require.Contains(t, outputString, randomIntegrationName) + }) + }) + + t.Run("create", func(t *testing.T) { + integrationYAMLPath := filepath.Join(t.TempDir(), "integration.yaml") + require.NoError(t, os.WriteFile(integrationYAMLPath, []byte(integrationYAML), 0644)) + _, err := runResourceCommand(t, fileConfig, []string{"create", integrationYAMLPath}) + require.NoError(t, err) + + buff, err := runResourceCommand(t, fileConfig, []string{"get", "integration/myawsint", "--format=text"}) + require.NoError(t, err) + outputString := buff.String() + require.Contains(t, outputString, "RoleARN=arn:aws:iam::123456789012:role/OpsTeam") + require.Contains(t, outputString, "myawsint") + + // Update the RoleARN to another role + integrationYAMLV2 := strings.ReplaceAll(integrationYAML, "OpsTeam", "DevTeam") + require.NoError(t, os.WriteFile(integrationYAMLPath, []byte(integrationYAMLV2), 0644)) + + // Trying to create it again should return an error + _, err = runResourceCommand(t, fileConfig, []string{"create", integrationYAMLPath}) + require.True(t, trace.IsAlreadyExists(err), "expected already exists error, got %v", err) + + // Using the force should be ok and replace the current object + _, err = runResourceCommand(t, fileConfig, []string{"create", "--force", integrationYAMLPath}) + require.NoError(t, err) + + // The RoleARN must be updated + buff, err = runResourceCommand(t, fileConfig, []string{"get", "integration/myawsint", "--format=text"}) + require.NoError(t, err) + outputString = buff.String() + require.Contains(t, outputString, "RoleARN=arn:aws:iam::123456789012:role/DevTeam") + }) +} + // TestAppResource tests tctl commands that manage application resources. func TestAppResource(t *testing.T) { dynAddr := newDynamicServiceAddr(t) @@ -564,6 +679,17 @@ spec: target: user: "bad@actor" message: "Come see me"` + + integrationYAML = `kind: integration +sub_kind: aws-oidc +version: v1 +metadata: + name: myawsint +spec: + subkind_spec: + aws_oidc: + role_arn: "arn:aws:iam::123456789012:role/OpsTeam" +` ) func TestCreateClusterAuthPreference_WithSupportForSecondFactorWithoutQuotes(t *testing.T) { From 2d41146ebfbedc0c2772b4b126bdce7b8314e641 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Thu, 6 Apr 2023 20:06:23 +0100 Subject: [PATCH 2/9] Add explicit type --- lib/web/ui/integration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web/ui/integration.go b/lib/web/ui/integration.go index 1ed22b427f0e9..7a83f32d3c3a8 100644 --- a/lib/web/ui/integration.go +++ b/lib/web/ui/integration.go @@ -76,7 +76,7 @@ func (r *UpdateIntegrationRequest) CheckAndSetDefaults() error { // a `nextToken` is provided and should be used to obtain the next page (as a query param `startKey`) type IntegrationsListResponse struct { // Items is a list of resources retrieved. - Items interface{} `json:"items"` + Items []Integration `json:"items"` // NextKey is the position to resume listing events. NextKey string `json:"nextKey"` } From 660ea87b8b721581e3b7d7ac912fcde4e27bc823 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 11 Apr 2023 08:04:29 +0100 Subject: [PATCH 3/9] add awsoidc role arn setter --- api/types/integration.go | 15 +++++++++++++++ lib/web/ui/integration.go | 8 ++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/api/types/integration.go b/api/types/integration.go index 5a996c5305a60..2fdd90e325786 100644 --- a/api/types/integration.go +++ b/api/types/integration.go @@ -38,6 +38,8 @@ type Integration interface { GetAWSOIDCIntegrationSpec() *AWSOIDCIntegrationSpecV1 // SetAWSOIDCIntegrationSpec sets the `aws-oidc` spec fields. SetAWSOIDCIntegrationSpec(*AWSOIDCIntegrationSpecV1) + // SetAWSOIDCRoleARN sets the RoleARN of the AWS OIDC Spec. + SetAWSOIDCRoleARN(string) } var _ ResourceWithLabels = (*IntegrationV1)(nil) @@ -136,6 +138,19 @@ func (ig *IntegrationV1) SetAWSOIDCIntegrationSpec(awsOIDCSpec *AWSOIDCIntegrati } } +// SetAWSOIDCRoleARN sets the RoleARN of the AWS OIDC Spec. +func (ig *IntegrationV1) SetAWSOIDCRoleARN(roleARN string) { + currentSubSpec := ig.Spec.GetAWSOIDC() + if currentSubSpec == nil { + currentSubSpec = &AWSOIDCIntegrationSpecV1{} + } + + currentSubSpec.RoleARN = roleARN + ig.Spec.SubKindSpec = &IntegrationSpecV1_AWSOIDC{ + AWSOIDC: currentSubSpec, + } +} + // Integrations is a list of Integration resources. type Integrations []Integration diff --git a/lib/web/ui/integration.go b/lib/web/ui/integration.go index 7a83f32d3c3a8..d385d9292f42d 100644 --- a/lib/web/ui/integration.go +++ b/lib/web/ui/integration.go @@ -31,11 +31,11 @@ type IntegrationAWSOIDCSpec struct { // Integration describes the base Integration fields (can be read or wri) Integration type Integration struct { // Name is the Integration name. - Name string `json:"name"` + Name string `json:"name,omitempty"` // SubKind is the Integration SubKind. - SubKind string `json:"subKind"` + SubKind string `json:"subKind,omitempty"` // AWSOIDC contains the fields for `aws-oidc` subkind integration. - AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc"` + AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc,omitempty"` } // CheckAndSetDefaults for the create request. @@ -59,7 +59,7 @@ func (r *Integration) CheckAndSetDefaults() error { // UpdateIntegrationRequest is a request to update an Integration type UpdateIntegrationRequest struct { // AWSOIDC contains the fields for `aws-oidc` subkind integration. - AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc"` + AWSOIDC *IntegrationAWSOIDCSpec `json:"awsoidc,omitempty"` } // CheckAndSetDefaults checks if the provided values are valid. From 7682c87acc44cac44b1b40f56a04d10f18260de4 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 11 Apr 2023 12:15:10 +0100 Subject: [PATCH 4/9] change serializer --- api/types/integration.go | 60 +++++++++++++----- integration/integrations/integration_test.go | 66 +++++++------------- lib/services/integration_test.go | 2 +- lib/web/integrations.go | 4 +- tool/tctl/common/collection.go | 2 +- tool/tctl/common/resource_command.go | 53 ++++++++-------- tool/tctl/common/resource_command_test.go | 5 +- 7 files changed, 99 insertions(+), 93 deletions(-) diff --git a/api/types/integration.go b/api/types/integration.go index 2fdd90e325786..0f18b03069533 100644 --- a/api/types/integration.go +++ b/api/types/integration.go @@ -34,6 +34,9 @@ const ( type Integration interface { ResourceWithLabels + // CanChangeStateTo checks if the current Integration can be updated for the provided integration. + CanChangeStateTo(Integration) error + // GetAWSOIDCIntegrationSpec returns the `aws-oidc` spec fields. GetAWSOIDCIntegrationSpec() *AWSOIDCIntegrationSpecV1 // SetAWSOIDCIntegrationSpec sets the `aws-oidc` spec fields. @@ -94,6 +97,19 @@ func (ig *IntegrationV1) CheckAndSetDefaults() error { return trace.Wrap(ig.Spec.CheckAndSetDefaults()) } +// CanChangeStateTo checks if the current Integration can be updated for the provided integration. +func (ig *IntegrationV1) CanChangeStateTo(newState Integration) error { + if ig.SubKind != newState.GetSubKind() { + return trace.BadParameter("cannot update %q fields for a %q integration", newState.GetSubKind(), ig.SubKind) + } + + if err := newState.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + return nil +} + // CheckAndSetDefaults validates and sets default values for a integration. func (s *IntegrationSpecV1) CheckAndSetDefaults() error { if s.SubKindSpec == nil { @@ -176,7 +192,7 @@ func (igs Integrations) Swap(i, j int) { igs[i], igs[j] = igs[j], igs[i] } // It is required because the Spec.SubKindSpec proto field is a oneof. // This translates into two issues when generating golang code: // - the Spec.SubKindSpec field in Go is an interface -// - there's no way to provide json tags for oneof fields, so instead of snake_case, we get CamelCase for the Spec.SubKindSpec field +// - it creates an extra field to store the oneof values // // Spec.SubKindSpec is an interface because it can have one of multiple values, // even though there's only one type for now: aws_oidc. @@ -185,16 +201,22 @@ func (igs Integrations) Swap(i, j int) { igs[i], igs[j] = igs[j], igs[i] } // and then use its SubKind to provide a concrete type for the Spec.SubKindSpec field. // Unmarshalling the remaining fields uses the standard json.Unmarshal over the Spec field. // -// Spec.SubKindSpec is expecting the `SubKindSpec` json tag, however we are using snake_case everywhere. -// So, we create a local type that has the expected json tag (`sub_kind_spec`) and use it to unmarshal and then copy -// to the proper type. +// Spec.SubKindSpec is an extra field which only adds clutter +// This method pulls those fields into a higher level. +// So, instead of: +// +// spec.subkind_spec.aws_oidc.role_arn: xyz +// +// It will be: +// +// spec.aws_oidc.role_arn: xyz func (ig *IntegrationV1) UnmarshalJSON(data []byte) error { var integration IntegrationV1 d := struct { ResourceHeader `json:""` Spec struct { - RawSubKindSpec json.RawMessage `json:"subkind_spec"` + AWSOIDC json.RawMessage `json:"aws_oidc"` } `json:"spec"` }{} @@ -205,20 +227,22 @@ func (ig *IntegrationV1) UnmarshalJSON(data []byte) error { integration.ResourceHeader = d.ResourceHeader - var subkindSpec isIntegrationSpecV1_SubKindSpec switch integration.SubKind { case IntegrationSubKindAWSOIDC: - subkindSpec = &IntegrationSpecV1_AWSOIDC{} + subkindSpec := &IntegrationSpecV1_AWSOIDC{ + AWSOIDC: &AWSOIDCIntegrationSpecV1{}, + } + + if err := json.Unmarshal(d.Spec.AWSOIDC, subkindSpec.AWSOIDC); err != nil { + return trace.Wrap(err) + } + + integration.Spec.SubKindSpec = subkindSpec + default: return trace.BadParameter("invalid subkind %q", integration.ResourceHeader.SubKind) } - if err := json.Unmarshal(d.Spec.RawSubKindSpec, subkindSpec); err != nil { - return trace.Wrap(err) - } - - integration.Spec.SubKindSpec = subkindSpec - if err := integration.CheckAndSetDefaults(); err != nil { return trace.Wrap(err) } @@ -235,12 +259,18 @@ func (ig *IntegrationV1) MarshalJSON() ([]byte, error) { d := struct { ResourceHeader `json:""` Spec struct { - SubKindSpec isIntegrationSpecV1_SubKindSpec `json:"subkind_spec"` + AWSOIDC AWSOIDCIntegrationSpecV1 `json:"aws_oidc"` } `json:"spec"` }{} d.ResourceHeader = ig.ResourceHeader - d.Spec.SubKindSpec = ig.Spec.SubKindSpec + + switch ig.SubKind { + case IntegrationSubKindAWSOIDC: + d.Spec.AWSOIDC = *ig.GetAWSOIDCIntegrationSpec() + default: + return nil, trace.BadParameter("invalid subkind %q", ig.SubKind) + } out, err := json.Marshal(d) return out, trace.Wrap(err) diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index ddfed549a8d2e..dea01fba39ac1 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -22,7 +22,7 @@ import ( "fmt" "io" "net/http" - "strings" + "net/url" "testing" "github.com/google/uuid" @@ -57,7 +57,8 @@ func TestIntegrationCRUD(t *testing.T) { require.NoError(t, err) require.NoError(t, authServer.UpsertRole(ctx, roleWithFullAccess)) - integrationsEndpoint := strings.Join([]string{"sites", "$site", "integrations"}, "/") + integrationsEndpoint, err := url.JoinPath("sites", "$site", "integrations") + require.NoError(t, err) // Set up User username := "fullaccess" @@ -74,12 +75,8 @@ func TestIntegrationCRUD(t *testing.T) { // List integrations should return empty resp, err := webPack.DoRequest(http.MethodGet, integrationsEndpoint, nil) + respBody := readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err := io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp := ui.IntegrationsListResponse{} @@ -97,22 +94,14 @@ func TestIntegrationCRUD(t *testing.T) { } resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Get One Integration by name resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp := ui.Integration{} @@ -132,12 +121,8 @@ func TestIntegrationCRUD(t *testing.T) { RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", }, }) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp = ui.Integration{} @@ -153,12 +138,8 @@ func TestIntegrationCRUD(t *testing.T) { // Delete resource resp, err = webPack.DoRequest(http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Add multiple integrations to test pagination @@ -175,23 +156,15 @@ func TestIntegrationCRUD(t *testing.T) { } resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) } // List integrations should return a full page resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10", nil) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -203,10 +176,7 @@ func TestIntegrationCRUD(t *testing.T) { resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) require.NoError(t, err) - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -216,12 +186,9 @@ func TestIntegrationCRUD(t *testing.T) { // Requesting the 3rd page should return a single item and empty StartKey resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) - respBody, err = io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -230,3 +197,12 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, 1) require.Empty(t, listResp.NextKey) } + +func readAllReadCloser(t *testing.T, r io.ReadCloser) []byte { + defer r.Close() + + respBody, err := io.ReadAll(r) + require.NoError(t, err) + + return respBody +} diff --git a/lib/services/integration_test.go b/lib/services/integration_test.go index d9016bb022fda..58c8103cdd896 100644 --- a/lib/services/integration_test.go +++ b/lib/services/integration_test.go @@ -50,7 +50,7 @@ func TestIntegrationUnmarshal(t *testing.T) { ) require.NoError(t, err) - storedBlob := []byte(`{"kind":"integration","sub_kind":"aws-oidc","version":"v1","metadata":{"name":"some-integration"},"spec":{"subkind_spec":{"aws_oidc":{"role_arn":"arn:aws:iam::123456789012:role/DevTeams"}}}}`) + storedBlob := []byte(`{"kind":"integration","sub_kind":"aws-oidc","version":"v1","metadata":{"name":"some-integration"},"spec":{"aws_oidc":{"role_arn":"arn:aws:iam::123456789012:role/DevTeams"}}}`) ig2, err := UnmarshalIntegration(storedBlob) require.NoError(t, err) diff --git a/lib/web/integrations.go b/lib/web/integrations.go index ecfd866a95b6a..89a550b91b515 100644 --- a/lib/web/integrations.go +++ b/lib/web/integrations.go @@ -107,9 +107,7 @@ func (h *Handler) integrationsUpdate(w http.ResponseWriter, r *http.Request, p h return nil, trace.BadParameter("cannot update %q fields for a %q integration", types.IntegrationSubKindAWSOIDC, integration.GetSubKind()) } - spec := integration.GetAWSOIDCIntegrationSpec() - spec.RoleARN = req.AWSOIDC.RoleARN - integration.SetAWSOIDCIntegrationSpec(spec) + integration.SetAWSOIDCRoleARN(req.AWSOIDC.RoleARN) } if _, err := clt.UpdateIntegration(r.Context(), integration); err != nil { diff --git a/tool/tctl/common/collection.go b/tool/tctl/common/collection.go index 0fd12bd12a72e..122ea8ca988a1 100644 --- a/tool/tctl/common/collection.go +++ b/tool/tctl/common/collection.go @@ -1006,7 +1006,7 @@ func (c *integrationCollection) writeText(w io.Writer) error { ig.GetName(), ig.GetSubKind(), strings.Join(specProps, ","), }) } - headers := []string{"Name", "SubKind", "Spec"} + headers := []string{"Name", "Type", "Spec"} t := asciitable.MakeTable(headers, rows...) _, err := t.AsBuffer().WriteTo(w) return trace.Wrap(err) diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index 650b49a98a8d6..982605b7f55a5 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -101,31 +101,31 @@ Same as above, but using JSON output: // Initialize allows ResourceCommand to plug itself into the CLI parser func (rc *ResourceCommand) Initialize(app *kingpin.Application, config *servicecfg.Config) { rc.CreateHandlers = map[ResourceKind]ResourceCreateHandler{ - types.KindUser: rc.createUser, - types.KindRole: rc.createRole, - types.KindTrustedCluster: rc.createTrustedCluster, - types.KindGithubConnector: rc.createGithubConnector, - types.KindCertAuthority: rc.createCertAuthority, - types.KindClusterAuthPreference: rc.createAuthPreference, - types.KindClusterNetworkingConfig: rc.createClusterNetworkingConfig, + types.KindUser: rc.createUser, + types.KindRole: rc.createRole, + types.KindTrustedCluster: rc.createTrustedCluster, + types.KindGithubConnector: rc.createGithubConnector, + types.KindCertAuthority: rc.createCertAuthority, + types.KindClusterAuthPreference: rc.createAuthPreference, + types.KindClusterNetworkingConfig: rc.createClusterNetworkingConfig, types.KindClusterMaintenanceConfig: rc.createClusterMaintenanceConfig, - types.KindSessionRecordingConfig: rc.createSessionRecordingConfig, - types.KindUIConfig: rc.createUIConfig, - types.KindLock: rc.createLock, - types.KindNetworkRestrictions: rc.createNetworkRestrictions, - types.KindApp: rc.createApp, - types.KindDatabase: rc.createDatabase, - types.KindKubernetesCluster: rc.createKubeCluster, - types.KindToken: rc.createToken, - types.KindInstaller: rc.createInstaller, - types.KindNode: rc.createNode, - types.KindOIDCConnector: rc.createOIDCConnector, - types.KindSAMLConnector: rc.createSAMLConnector, - types.KindLoginRule: rc.createLoginRule, - types.KindSAMLIdPServiceProvider: rc.createSAMLIdPServiceProvider, - types.KindDevice: rc.createDevice, - types.KindOktaImportRule: rc.createOktaImportRule, - types.KindIntegration: rc.createIntegration, + types.KindSessionRecordingConfig: rc.createSessionRecordingConfig, + types.KindUIConfig: rc.createUIConfig, + types.KindLock: rc.createLock, + types.KindNetworkRestrictions: rc.createNetworkRestrictions, + types.KindApp: rc.createApp, + types.KindDatabase: rc.createDatabase, + types.KindKubernetesCluster: rc.createKubeCluster, + types.KindToken: rc.createToken, + types.KindInstaller: rc.createInstaller, + types.KindNode: rc.createNode, + types.KindOIDCConnector: rc.createOIDCConnector, + types.KindSAMLConnector: rc.createSAMLConnector, + types.KindLoginRule: rc.createLoginRule, + types.KindSAMLIdPServiceProvider: rc.createSAMLIdPServiceProvider, + types.KindDevice: rc.createDevice, + types.KindOktaImportRule: rc.createOktaImportRule, + types.KindIntegration: rc.createIntegration, } rc.config = config @@ -890,10 +890,13 @@ func (rc *ResourceCommand) createIntegration(ctx context.Context, client auth.Cl return trace.AlreadyExists("Integration %q already exists", integration.GetName()) } + if err := existingIntegration.CanChangeStateTo(integration); err != nil { + return trace.Wrap(err) + } + switch integration.GetSubKind() { case types.IntegrationSubKindAWSOIDC: existingIntegration.SetAWSOIDCIntegrationSpec(integration.GetAWSOIDCIntegrationSpec()) - default: return trace.BadParameter("subkind %q is not supported", integration.GetSubKind()) } diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index 7339065cc44f2..8362fe0fd614f 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -686,9 +686,8 @@ version: v1 metadata: name: myawsint spec: - subkind_spec: - aws_oidc: - role_arn: "arn:aws:iam::123456789012:role/OpsTeam" + aws_oidc: + role_arn: "arn:aws:iam::123456789012:role/OpsTeam" ` ) From 673c2285fc22e2d625d47fb2bb59c14dab621df6 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 11 Apr 2023 14:20:05 +0100 Subject: [PATCH 5/9] ignore bodyclose linter false positive --- integration/integrations/integration_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index dea01fba39ac1..1c2b8b9e2685d 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -74,6 +74,7 @@ func TestIntegrationCRUD(t *testing.T) { webPack := helpers.LoginWebClient(t, proxyAddr.String(), username, userPassword) // List integrations should return empty + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err := webPack.DoRequest(http.MethodGet, integrationsEndpoint, nil) respBody := readAllReadCloser(t, resp.Body) require.NoError(t, err) @@ -93,12 +94,14 @@ func TestIntegrationCRUD(t *testing.T) { }, } + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Get One Integration by name + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) @@ -116,6 +119,7 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Update the integration to another RoleARN + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ AWSOIDC: &ui.IntegrationAWSOIDCSpec{ RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", @@ -137,6 +141,7 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Delete resource + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) @@ -155,13 +160,15 @@ func TestIntegrationCRUD(t *testing.T) { }, } - resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) + //nolint:bodyclose // Body is closed in readAllReadCloser + resp, err := webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) } // List integrations should return a full page + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10", nil) respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) @@ -173,6 +180,7 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 2nd page should return a full page + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) require.NoError(t, err) @@ -185,8 +193,11 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 3rd page should return a single item and empty StartKey + //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - respBody = readAllReadCloser(t, resp.Body) + respBody, errReadAll := io.ReadAll(resp.Body) + require.NoError(t, errReadAll) + resp.Body.Close() require.NoError(t, err) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) From d1c340d3485a9cd06e268c0148087c0e738c69f1 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 11 Apr 2023 19:11:03 +0100 Subject: [PATCH 6/9] check for error before reading --- api/types/integration.go | 4 ++++ integration/integrations/integration_test.go | 19 +++++++++---------- lib/web/ui/integration.go | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/api/types/integration.go b/api/types/integration.go index 0f18b03069533..92590ad7fde77 100644 --- a/api/types/integration.go +++ b/api/types/integration.go @@ -267,6 +267,10 @@ func (ig *IntegrationV1) MarshalJSON() ([]byte, error) { switch ig.SubKind { case IntegrationSubKindAWSOIDC: + if ig.GetAWSOIDCIntegrationSpec() == nil { + return nil, trace.BadParameter("missing subkind data for %q subkind", ig.SubKind) + } + d.Spec.AWSOIDC = *ig.GetAWSOIDCIntegrationSpec() default: return nil, trace.BadParameter("invalid subkind %q", ig.SubKind) diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index 1c2b8b9e2685d..55a52b1b34c36 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -76,8 +76,8 @@ func TestIntegrationCRUD(t *testing.T) { // List integrations should return empty //nolint:bodyclose // Body is closed in readAllReadCloser resp, err := webPack.DoRequest(http.MethodGet, integrationsEndpoint, nil) - respBody := readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody := readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp := ui.IntegrationsListResponse{} @@ -96,15 +96,15 @@ func TestIntegrationCRUD(t *testing.T) { //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Get One Integration by name //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp := ui.Integration{} @@ -125,8 +125,8 @@ func TestIntegrationCRUD(t *testing.T) { RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", }, }) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp = ui.Integration{} @@ -143,8 +143,8 @@ func TestIntegrationCRUD(t *testing.T) { // Delete resource //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Add multiple integrations to test pagination @@ -162,16 +162,16 @@ func TestIntegrationCRUD(t *testing.T) { //nolint:bodyclose // Body is closed in readAllReadCloser resp, err := webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) } // List integrations should return a full page //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10", nil) - respBody = readAllReadCloser(t, resp.Body) require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -183,7 +183,6 @@ func TestIntegrationCRUD(t *testing.T) { //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) @@ -195,8 +194,8 @@ func TestIntegrationCRUD(t *testing.T) { // Requesting the 3rd page should return a single item and empty StartKey //nolint:bodyclose // Body is closed in readAllReadCloser resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - respBody, errReadAll := io.ReadAll(resp.Body) - require.NoError(t, errReadAll) + require.NoError(t, err) + respBody = readAllReadCloser(t, resp.Body) resp.Body.Close() require.NoError(t, err) diff --git a/lib/web/ui/integration.go b/lib/web/ui/integration.go index d385d9292f42d..3078745dd7aae 100644 --- a/lib/web/ui/integration.go +++ b/lib/web/ui/integration.go @@ -28,7 +28,7 @@ type IntegrationAWSOIDCSpec struct { RoleARN string `json:"roleArn,omitempty"` } -// Integration describes the base Integration fields (can be read or wri) Integration +// Integration describes Integration fields type Integration struct { // Name is the Integration name. Name string `json:"name,omitempty"` From c3415881a2360e7e30f674b04ebaffd465442e8a Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 12 Apr 2023 15:47:30 +0100 Subject: [PATCH 7/9] simplify webPack.DoRequest call --- integration/conntest/database_test.go | 16 +----- integration/db/database_service_test.go | 9 +-- integration/helpers/web.go | 32 +++++------ integration/integrations/integration_test.go | 60 ++++---------------- 4 files changed, 26 insertions(+), 91 deletions(-) diff --git a/integration/conntest/database_test.go b/integration/conntest/database_test.go index a195a6a6d200b..8c6e6ab9e62b4 100644 --- a/integration/conntest/database_test.go +++ b/integration/conntest/database_test.go @@ -18,7 +18,6 @@ import ( "context" "encoding/base32" "encoding/json" - "io" "net" "net/http" "strings" @@ -216,13 +215,7 @@ func TestDiagnoseConnectionForPostgresDatabases(t *testing.T) { DialTimeout: time.Second, InsecureSkipVerify: true, } - resp, err := webPack.DoRequest(http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) - require.NoError(t, err) - - respBody, err := io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() + resp, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) var connectionDiagnostic ui.ConnectionDiagnostic @@ -307,12 +300,7 @@ func TestDiagnoseConnectionForPostgresDatabases(t *testing.T) { TOTPCode: validToken, }, } - resp, err := webPack.DoRequest(http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) - require.NoError(t, err) - respBody, err := io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() + resp, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) var connectionDiagnostic ui.ConnectionDiagnostic diff --git a/integration/db/database_service_test.go b/integration/db/database_service_test.go index 2bc35d2a246e4..d3425e5561816 100644 --- a/integration/db/database_service_test.go +++ b/integration/db/database_service_test.go @@ -17,7 +17,6 @@ package db import ( "context" "encoding/json" - "io" "net/http" "strings" "testing" @@ -85,13 +84,7 @@ func TestDatabaseServiceHeartbeat(t *testing.T) { // List Database Services listDBServicesEndpoint := strings.Join([]string{"sites", "$site", "databaseservices"}, "/") - resp, err := webPack.DoRequest(http.MethodGet, listDBServicesEndpoint, nil) - require.NoError(t, err) - - respBody, err := io.ReadAll(resp.Body) - require.NoError(t, err) - - defer resp.Body.Close() + resp, respBody := webPack.DoRequest(t, http.MethodGet, listDBServicesEndpoint, nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) var listResp listDatabaseServicesResp diff --git a/integration/helpers/web.go b/integration/helpers/web.go index 17449991c108a..c95c6e381cdf5 100644 --- a/integration/helpers/web.go +++ b/integration/helpers/web.go @@ -25,7 +25,6 @@ import ( "strings" "testing" - "github.com/gravitational/trace" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/lib/httplib/csrf" @@ -101,13 +100,7 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac bearerToken: csResp.Token, } - resp, err = webClient.DoRequest(http.MethodGet, "sites", nil) - require.NoError(t, err) - defer resp.Body.Close() - - bs, err := io.ReadAll(resp.Body) - require.NoError(t, err) - + resp, bs := webClient.DoRequest(t, http.MethodGet, "sites", nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(bs)) var clusters []ui.Cluster @@ -121,23 +114,17 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac // DoRequest receives a method, endpoint and payload and sends an HTTP Request to the Teleport API. // The endpoint must not contain the host neither the base path ('/v1/webapi/'). // Returns the http.Response. -func (w *WebClientPack) DoRequest(method, endpoint string, payload any) (*http.Response, error) { +func (w *WebClientPack) DoRequest(t *testing.T, method, endpoint string, payload any) (*http.Response, []byte) { endpoint = fmt.Sprintf("https://%s/v1/webapi/%s", w.host, endpoint) endpoint = strings.ReplaceAll(endpoint, "$site", w.clusterName) u, err := url.Parse(endpoint) - if err != nil { - return nil, trace.Wrap(err) - } + require.NoError(t, err) bs, err := json.Marshal(payload) - if err != nil { - return nil, trace.Wrap(err) - } + require.NoError(t, err) req, err := http.NewRequest(method, u.String(), bytes.NewBuffer(bs)) - if err != nil { - return nil, trace.Wrap(err) - } + require.NoError(t, err) req.AddCookie(&http.Cookie{ Name: web.CookieName, @@ -147,5 +134,12 @@ func (w *WebClientPack) DoRequest(method, endpoint string, payload any) (*http.R req.Header.Add("Content-Type", "application/json") resp, err := w.clt.Do(req) - return resp, trace.Wrap(err) + require.NoError(t, err) + + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return resp, body } diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index 55a52b1b34c36..e77b5059e7c2d 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "net/url" "testing" @@ -35,7 +34,7 @@ import ( "github.com/gravitational/teleport/lib/web/ui" ) -// TestIntegraitionCRUD starts a Teleport cluster and using its Proxy Web server, +// TestIntegrationCRUD starts a Teleport cluster and using its Proxy Web server, // tests the CRUD operations over the Integration resource. func TestIntegrationCRUD(t *testing.T) { ctx := context.Background() @@ -74,10 +73,7 @@ func TestIntegrationCRUD(t *testing.T) { webPack := helpers.LoginWebClient(t, proxyAddr.String(), username, userPassword) // List integrations should return empty - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err := webPack.DoRequest(http.MethodGet, integrationsEndpoint, nil) - require.NoError(t, err) - respBody := readAllReadCloser(t, resp.Body) + resp, respBody := webPack.DoRequest(t, http.MethodGet, integrationsEndpoint, nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp := ui.IntegrationsListResponse{} @@ -94,17 +90,11 @@ func TestIntegrationCRUD(t *testing.T) { }, } - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody = webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Get One Integration by name - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp := ui.Integration{} @@ -119,14 +109,11 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Update the integration to another RoleARN - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ + resp, respBody = webPack.DoRequest(t, http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ AWSOIDC: &ui.IntegrationAWSOIDCSpec{ RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", }, }) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) integrationResp = ui.Integration{} @@ -141,10 +128,7 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Delete resource - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody = webPack.DoRequest(t, http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) // Add multiple integrations to test pagination @@ -160,18 +144,12 @@ func TestIntegrationCRUD(t *testing.T) { }, } - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err := webPack.DoRequest(http.MethodPost, integrationsEndpoint, createIntegrationReq) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody := webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) } // List integrations should return a full page - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10", nil) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10", nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -180,10 +158,7 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 2nd page should return a full page - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) + resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -192,13 +167,7 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 3rd page should return a single item and empty StartKey - //nolint:bodyclose // Body is closed in readAllReadCloser - resp, err = webPack.DoRequest(http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.NoError(t, err) - respBody = readAllReadCloser(t, resp.Body) - resp.Body.Close() - require.NoError(t, err) - + resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} @@ -207,12 +176,3 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, 1) require.Empty(t, listResp.NextKey) } - -func readAllReadCloser(t *testing.T, r io.ReadCloser) []byte { - defer r.Close() - - respBody, err := io.ReadAll(r) - require.NoError(t, err) - - return respBody -} From cba07b5808fa211fec6baa06ddeef127775d1e3e Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 12 Apr 2023 16:27:59 +0100 Subject: [PATCH 8/9] fix godoc of WebClientPack.DoRequest --- integration/helpers/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/helpers/web.go b/integration/helpers/web.go index c95c6e381cdf5..61df1af66db88 100644 --- a/integration/helpers/web.go +++ b/integration/helpers/web.go @@ -113,7 +113,7 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac // DoRequest receives a method, endpoint and payload and sends an HTTP Request to the Teleport API. // The endpoint must not contain the host neither the base path ('/v1/webapi/'). -// Returns the http.Response. +// Body is read and returned (as []bytes) as well as the http.Response. func (w *WebClientPack) DoRequest(t *testing.T, method, endpoint string, payload any) (*http.Response, []byte) { endpoint = fmt.Sprintf("https://%s/v1/webapi/%s", w.host, endpoint) endpoint = strings.ReplaceAll(endpoint, "$site", w.clusterName) From 0ecd29566c9c0a6a16eafc6e53b4578a904917fd Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 12 Apr 2023 16:46:12 +0100 Subject: [PATCH 9/9] return body and status code only --- integration/conntest/database_test.go | 8 ++--- integration/db/database_service_test.go | 4 +-- integration/helpers/web.go | 10 +++--- integration/integrations/integration_test.go | 36 ++++++++++---------- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/integration/conntest/database_test.go b/integration/conntest/database_test.go index 8c6e6ab9e62b4..f8ec75643efcb 100644 --- a/integration/conntest/database_test.go +++ b/integration/conntest/database_test.go @@ -215,8 +215,8 @@ func TestDiagnoseConnectionForPostgresDatabases(t *testing.T) { DialTimeout: time.Second, InsecureSkipVerify: true, } - resp, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) var connectionDiagnostic ui.ConnectionDiagnostic require.NoError(t, json.Unmarshal(respBody, &connectionDiagnostic)) @@ -300,8 +300,8 @@ func TestDiagnoseConnectionForPostgresDatabases(t *testing.T) { TOTPCode: validToken, }, } - resp, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody := webPack.DoRequest(t, http.MethodPost, diagnoseConnectionEndpoint, diagnoseReq) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) var connectionDiagnostic ui.ConnectionDiagnostic require.NoError(t, json.Unmarshal(respBody, &connectionDiagnostic)) diff --git a/integration/db/database_service_test.go b/integration/db/database_service_test.go index d3425e5561816..002e2ec74a818 100644 --- a/integration/db/database_service_test.go +++ b/integration/db/database_service_test.go @@ -84,8 +84,8 @@ func TestDatabaseServiceHeartbeat(t *testing.T) { // List Database Services listDBServicesEndpoint := strings.Join([]string{"sites", "$site", "databaseservices"}, "/") - resp, respBody := webPack.DoRequest(t, http.MethodGet, listDBServicesEndpoint, nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody := webPack.DoRequest(t, http.MethodGet, listDBServicesEndpoint, nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) var listResp listDatabaseServicesResp require.NoError(t, json.Unmarshal(respBody, &listResp)) diff --git a/integration/helpers/web.go b/integration/helpers/web.go index 61df1af66db88..e1cfbc85cb4ac 100644 --- a/integration/helpers/web.go +++ b/integration/helpers/web.go @@ -100,8 +100,8 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac bearerToken: csResp.Token, } - resp, bs := webClient.DoRequest(t, http.MethodGet, "sites", nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(bs)) + respStatusCode, bs := webClient.DoRequest(t, http.MethodGet, "sites", nil) + require.Equal(t, http.StatusOK, respStatusCode, string(bs)) var clusters []ui.Cluster require.NoError(t, json.Unmarshal(bs, &clusters), string(bs)) @@ -113,8 +113,8 @@ func LoginWebClient(t *testing.T, host, username, password string) *WebClientPac // DoRequest receives a method, endpoint and payload and sends an HTTP Request to the Teleport API. // The endpoint must not contain the host neither the base path ('/v1/webapi/'). -// Body is read and returned (as []bytes) as well as the http.Response. -func (w *WebClientPack) DoRequest(t *testing.T, method, endpoint string, payload any) (*http.Response, []byte) { +// Status Code and Body are returned. +func (w *WebClientPack) DoRequest(t *testing.T, method, endpoint string, payload any) (int, []byte) { endpoint = fmt.Sprintf("https://%s/v1/webapi/%s", w.host, endpoint) endpoint = strings.ReplaceAll(endpoint, "$site", w.clusterName) u, err := url.Parse(endpoint) @@ -141,5 +141,5 @@ func (w *WebClientPack) DoRequest(t *testing.T, method, endpoint string, payload body, err := io.ReadAll(resp.Body) require.NoError(t, err) - return resp, body + return resp.StatusCode, body } diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index e77b5059e7c2d..30780367a179c 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -73,8 +73,8 @@ func TestIntegrationCRUD(t *testing.T) { webPack := helpers.LoginWebClient(t, proxyAddr.String(), username, userPassword) // List integrations should return empty - resp, respBody := webPack.DoRequest(t, http.MethodGet, integrationsEndpoint, nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody := webPack.DoRequest(t, http.MethodGet, integrationsEndpoint, nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) listResp := ui.IntegrationsListResponse{} require.NoError(t, json.Unmarshal(respBody, &listResp)) @@ -90,12 +90,12 @@ func TestIntegrationCRUD(t *testing.T) { }, } - resp, respBody = webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) // Get One Integration by name - resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"/MyAWSAccount", nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) integrationResp := ui.Integration{} require.NoError(t, json.Unmarshal(respBody, &integrationResp)) @@ -109,12 +109,12 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Update the integration to another RoleARN - resp, respBody = webPack.DoRequest(t, http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ + respStatusCode, respBody = webPack.DoRequest(t, http.MethodPut, integrationsEndpoint+"/MyAWSAccount", ui.UpdateIntegrationRequest{ AWSOIDC: &ui.IntegrationAWSOIDCSpec{ RoleARN: "arn:aws:iam::123456789012:role/OpsTeam", }, }) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) integrationResp = ui.Integration{} require.NoError(t, json.Unmarshal(respBody, &integrationResp)) @@ -128,8 +128,8 @@ func TestIntegrationCRUD(t *testing.T) { }, integrationResp, string(respBody)) // Delete resource - resp, respBody = webPack.DoRequest(t, http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodDelete, integrationsEndpoint+"/MyAWSAccount", nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) // Add multiple integrations to test pagination // Tests two full pages + 1 item to prevent off by one errors. @@ -144,13 +144,13 @@ func TestIntegrationCRUD(t *testing.T) { }, } - resp, respBody := webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody := webPack.DoRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationReq) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) } // List integrations should return a full page - resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10", nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10", nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} require.NoError(t, json.Unmarshal(respBody, &listResp)) @@ -158,8 +158,8 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 2nd page should return a full page - resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} require.NoError(t, json.Unmarshal(respBody, &listResp)) @@ -167,8 +167,8 @@ func TestIntegrationCRUD(t *testing.T) { require.Len(t, listResp.Items, pageSize) // Requesting the 3rd page should return a single item and empty StartKey - resp, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) + respStatusCode, respBody = webPack.DoRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) listResp = ui.IntegrationsListResponse{} require.NoError(t, json.Unmarshal(respBody, &listResp))