From fba36e54119215554408dfb8e798565a16c3ea81 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Mon, 12 Aug 2024 15:40:49 +0100 Subject: [PATCH] Add tags to SSM Doc when configuring the AWS OIDC EC2 flow (#44812) --- lib/config/configuration.go | 6 +++ .../awsoidc/ec2_ssm_iam_config.go | 19 +++++++++ .../awsoidc/ec2_ssm_iam_config_test.go | 40 ++++++++++++++++++- lib/integrations/awsoidc/tags/tags.go | 14 +++++++ lib/integrations/awsoidc/tags/tags_test.go | 10 +++++ lib/web/integrations_awsoidc.go | 12 ++++++ lib/web/integrations_awsoidc_test.go | 22 ++++++---- tool/teleport/common/integration_configure.go | 2 + tool/teleport/common/teleport.go | 2 + .../DiscoveryConfigSsm/DiscoveryConfigSsm.tsx | 1 + web/packages/teleport/src/config.ts | 3 +- 11 files changed, 120 insertions(+), 11 deletions(-) diff --git a/lib/config/configuration.go b/lib/config/configuration.go index d614eeb476f81..43df96534b65f 100644 --- a/lib/config/configuration.go +++ b/lib/config/configuration.go @@ -327,6 +327,12 @@ type IntegrationConfEC2SSMIAM struct { // No trailing / is expected. // Eg https://tenant.teleport.sh ProxyPublicURL string + // ClusterName is the Teleport cluster name. + // Used for resource tagging. + ClusterName string + // IntegrationName is the Teleport AWS OIDC Integration name. + // Used for resource tagging. + IntegrationName string } // IntegrationConfEKSIAM contains the arguments of diff --git a/lib/integrations/awsoidc/ec2_ssm_iam_config.go b/lib/integrations/awsoidc/ec2_ssm_iam_config.go index 8f1903bd19b47..55ebcccaa0a03 100644 --- a/lib/integrations/awsoidc/ec2_ssm_iam_config.go +++ b/lib/integrations/awsoidc/ec2_ssm_iam_config.go @@ -31,6 +31,7 @@ import ( "github.com/gravitational/trace" awslib "github.com/gravitational/teleport/lib/cloud/aws" + "github.com/gravitational/teleport/lib/integrations/awsoidc/tags" ) const ( @@ -60,6 +61,13 @@ type EC2SSMIAMConfigureRequest struct { // No trailing / is expected. // Eg https://tenant.teleport.sh ProxyPublicURL string + + // ClusterName is the Teleport cluster name. + // Used for resource tagging. + ClusterName string + // IntegrationName is the Teleport AWS OIDC Integration name. + // Used for resource tagging. + IntegrationName string } // CheckAndSetDefaults ensures the required fields are present. @@ -84,6 +92,14 @@ func (r *EC2SSMIAMConfigureRequest) CheckAndSetDefaults() error { return trace.BadParameter("proxy public url is required") } + if r.ClusterName == "" { + return trace.BadParameter("cluster name is required") + } + + if r.IntegrationName == "" { + return trace.BadParameter("integration name is required") + } + return nil } @@ -165,11 +181,14 @@ func ConfigureEC2SSM(ctx context.Context, clt EC2SSMConfigureClient, req EC2SSMI slog.InfoContext(ctx, "IntegrationRole: IAM Policy added to Role", "policy", req.IntegrationRoleEC2SSMPolicy, "role", req.IntegrationRole) + ownershipTags := tags.DefaultResourceCreationTags(req.ClusterName, req.IntegrationName) + _, err = clt.CreateDocument(ctx, &ssm.CreateDocumentInput{ Name: aws.String(req.SSMDocumentName), DocumentType: ssmtypes.DocumentTypeCommand, DocumentFormat: ssmtypes.DocumentFormatYaml, Content: aws.String(awslib.EC2DiscoverySSMDocument(req.ProxyPublicURL)), + Tags: ownershipTags.ToSSMTags(), }) if err != nil { var docAlreadyExistsError *ssmtypes.DocumentAlreadyExists diff --git a/lib/integrations/awsoidc/ec2_ssm_iam_config_test.go b/lib/integrations/awsoidc/ec2_ssm_iam_config_test.go index 1e2828be03785..298a464a10492 100644 --- a/lib/integrations/awsoidc/ec2_ssm_iam_config_test.go +++ b/lib/integrations/awsoidc/ec2_ssm_iam_config_test.go @@ -39,6 +39,8 @@ func TestEC2SSMIAMConfigReqDefaults(t *testing.T) { IntegrationRole: "integrationrole", SSMDocumentName: "MyDoc", ProxyPublicURL: "https://proxy.example.com", + ClusterName: "my-cluster", + IntegrationName: "my-integration", } } @@ -58,6 +60,8 @@ func TestEC2SSMIAMConfigReqDefaults(t *testing.T) { IntegrationRoleEC2SSMPolicy: "EC2DiscoverWithSSM", SSMDocumentName: "MyDoc", ProxyPublicURL: "https://proxy.example.com", + ClusterName: "my-cluster", + IntegrationName: "my-integration", }, }, { @@ -78,6 +82,24 @@ func TestEC2SSMIAMConfigReqDefaults(t *testing.T) { }, errCheck: badParameterCheck, }, + { + name: "missing integration name", + req: func() EC2SSMIAMConfigureRequest { + req := baseReq() + req.IntegrationName = "" + return req + }, + errCheck: badParameterCheck, + }, + { + name: "missing cluster name", + req: func() EC2SSMIAMConfigureRequest { + req := baseReq() + req.ClusterName = "" + return req + }, + errCheck: badParameterCheck, + }, { name: "missing ssm document", req: func() EC2SSMIAMConfigureRequest { @@ -118,6 +140,8 @@ func TestEC2SSMIAMConfig(t *testing.T) { IntegrationRole: "integrationrole", SSMDocumentName: "MyDoc", ProxyPublicURL: "https://proxy.example.com", + ClusterName: "my-cluster", + IntegrationName: "my-integration", } } @@ -157,13 +181,21 @@ func TestEC2SSMIAMConfig(t *testing.T) { err := ConfigureEC2SSM(ctx, &clt, tt.req()) tt.errCheck(t, err) + if err == nil { + require.Contains(t, clt.existingDocs, tt.req().SSMDocumentName) + require.ElementsMatch(t, []ssmtypes.Tag{ + {Key: aws.String("teleport.dev/cluster"), Value: aws.String("my-cluster")}, + {Key: aws.String("teleport.dev/integration"), Value: aws.String("my-integration")}, + {Key: aws.String("teleport.dev/origin"), Value: aws.String("integration_awsoidc")}, + }, clt.existingDocs[tt.req().SSMDocumentName]) + } }) } } type mockEC2SSMIAMConfigClient struct { existingRoles []string - existingDocs []string + existingDocs map[string][]ssmtypes.Tag } // PutRolePolicy creates or replaces a Policy by its name in a IAM Role. @@ -179,8 +211,12 @@ func (m *mockEC2SSMIAMConfigClient) PutRolePolicy(ctx context.Context, params *i // CreateDocument creates an SSM document. func (m *mockEC2SSMIAMConfigClient) CreateDocument(ctx context.Context, params *ssm.CreateDocumentInput, optFns ...func(*ssm.Options)) (*ssm.CreateDocumentOutput, error) { - if slices.Contains(m.existingDocs, aws.ToString(params.Name)) { + if m.existingDocs == nil { + m.existingDocs = make(map[string][]ssmtypes.Tag) + } + if _, ok := m.existingDocs[aws.ToString(params.Name)]; ok { return nil, &ssmtypes.DocumentAlreadyExists{} } + m.existingDocs[aws.ToString(params.Name)] = params.Tags return nil, nil } diff --git a/lib/integrations/awsoidc/tags/tags.go b/lib/integrations/awsoidc/tags/tags.go index 9ec8c7646f851..c04735f02878c 100644 --- a/lib/integrations/awsoidc/tags/tags.go +++ b/lib/integrations/awsoidc/tags/tags.go @@ -28,6 +28,7 @@ import ( ecsTypes "github.com/aws/aws-sdk-go-v2/service/ecs/types" iamTypes "github.com/aws/aws-sdk-go-v2/service/iam/types" s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" + ssmtypes "github.com/aws/aws-sdk-go-v2/service/ssm/types" "github.com/gravitational/teleport/api/types" ) @@ -157,6 +158,19 @@ func (d AWSTags) ToAthenaTags() []athenatypes.Tag { return athenaTags } +// ToSSMTags returns the default tags using the expected type for SSM resources: [ssmtypes.Tag] +func (d AWSTags) ToSSMTags() []ssmtypes.Tag { + ssmTags := make([]ssmtypes.Tag, 0, len(d)) + for k, v := range d { + k, v := k, v + ssmTags = append(ssmTags, ssmtypes.Tag{ + Key: &k, + Value: &v, + }) + } + return ssmTags +} + // ToMap returns the default tags using the expected type for other aws resources. // Eg Glue resources func (d AWSTags) ToMap() map[string]string { diff --git a/lib/integrations/awsoidc/tags/tags_test.go b/lib/integrations/awsoidc/tags/tags_test.go index 7b07e86f9340e..287c4caebe3a6 100644 --- a/lib/integrations/awsoidc/tags/tags_test.go +++ b/lib/integrations/awsoidc/tags/tags_test.go @@ -25,6 +25,7 @@ import ( ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types" iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" + ssmtypes "github.com/aws/aws-sdk-go-v2/service/ssm/types" "github.com/stretchr/testify/require" ) @@ -67,6 +68,15 @@ func TestDefaultTags(t *testing.T) { require.ElementsMatch(t, expectedEC2Tags, d.ToEC2Tags()) }) + t.Run("ssm tags", func(t *testing.T) { + expectedTags := []ssmtypes.Tag{ + {Key: aws.String("teleport.dev/cluster"), Value: aws.String("mycluster")}, + {Key: aws.String("teleport.dev/integration"), Value: aws.String("myawsaccount")}, + {Key: aws.String("teleport.dev/origin"), Value: aws.String("integration_awsoidc")}, + } + require.ElementsMatch(t, expectedTags, d.ToSSMTags()) + }) + t.Run("resource is teleport managed", func(t *testing.T) { t.Run("ECS Tags", func(t *testing.T) { t.Run("all tags match", func(t *testing.T) { diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 4a4e91a74bef1..3ad0149c86b96 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -369,6 +369,11 @@ func (h *Handler) awsOIDCConfigureAWSAppAccessIAM(w http.ResponseWriter, r *http func (h *Handler) awsOIDCConfigureEC2SSMIAM(w http.ResponseWriter, r *http.Request, p httprouter.Params) (any, error) { queryParams := r.URL.Query() + integrationName := queryParams.Get("integrationName") + if len(integrationName) == 0 { + return nil, trace.BadParameter("missing integrationName param") + } + role := queryParams.Get("role") if err := aws.IsValidIAMRoleName(role); err != nil { return nil, trace.BadParameter("invalid role %q", role) @@ -390,6 +395,11 @@ func (h *Handler) awsOIDCConfigureEC2SSMIAM(w http.ResponseWriter, r *http.Reque proxyPublicURL = "https://" + proxyPublicURL } + clusterName, err := h.GetProxyClient().GetDomainName(r.Context()) + if err != nil { + return nil, trace.Wrap(err) + } + // The script must execute the following command: // teleport integration configure ec2-ssm-iam argsList := []string{ @@ -398,6 +408,8 @@ func (h *Handler) awsOIDCConfigureEC2SSMIAM(w http.ResponseWriter, r *http.Reque fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(region)), fmt.Sprintf("--ssm-document-name=%s", shsprintf.EscapeDefaultContext(ssmDocumentName)), fmt.Sprintf("--proxy-public-url=%s", shsprintf.EscapeDefaultContext(proxyPublicURL)), + fmt.Sprintf("--cluster=%s", shsprintf.EscapeDefaultContext(clusterName)), + fmt.Sprintf("--name=%s", shsprintf.EscapeDefaultContext(integrationName)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), diff --git a/lib/web/integrations_awsoidc_test.go b/lib/web/integrations_awsoidc_test.go index ff16e83af6b8d..ceeed0eb52f0d 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -284,30 +284,36 @@ func TestBuildEC2SSMIAMScript(t *testing.T) { { name: "valid", reqQuery: url.Values{ - "awsRegion": []string{"us-east-1"}, - "role": []string{"myRole"}, - "ssmDocument": []string{"TeleportDiscoveryInstallerTest"}, + "awsRegion": []string{"us-east-1"}, + "role": []string{"myRole"}, + "ssmDocument": []string{"TeleportDiscoveryInstallerTest"}, + "integrationName": []string{"my-integration"}, }, errCheck: require.NoError, expectedTeleportArgs: "integration configure ec2-ssm-iam " + "--role=myRole " + "--aws-region=us-east-1 " + "--ssm-document-name=TeleportDiscoveryInstallerTest " + - "--proxy-public-url=" + proxyPublicURL, + "--proxy-public-url=" + proxyPublicURL + " " + + "--cluster=localhost " + + "--name=my-integration", }, { name: "valid with symbols in role", reqQuery: url.Values{ - "awsRegion": []string{"us-east-1"}, - "role": []string{"Test+1=2,3.4@5-6_7"}, - "ssmDocument": []string{"TeleportDiscoveryInstallerTest"}, + "awsRegion": []string{"us-east-1"}, + "role": []string{"Test+1=2,3.4@5-6_7"}, + "ssmDocument": []string{"TeleportDiscoveryInstallerTest"}, + "integrationName": []string{"my-integration"}, }, errCheck: require.NoError, expectedTeleportArgs: "integration configure ec2-ssm-iam " + "--role=Test\\+1=2,3.4\\@5-6_7 " + "--aws-region=us-east-1 " + "--ssm-document-name=TeleportDiscoveryInstallerTest " + - "--proxy-public-url=" + proxyPublicURL, + "--proxy-public-url=" + proxyPublicURL + " " + + "--cluster=localhost " + + "--name=my-integration", }, { name: "missing aws-region", diff --git a/tool/teleport/common/integration_configure.go b/tool/teleport/common/integration_configure.go index dab36f0a5f8b3..ca2296555e3cf 100644 --- a/tool/teleport/common/integration_configure.go +++ b/tool/teleport/common/integration_configure.go @@ -92,6 +92,8 @@ func onIntegrationConfEC2SSMIAM(ctx context.Context, params config.IntegrationCo IntegrationRole: params.RoleName, SSMDocumentName: params.SSMDocumentName, ProxyPublicURL: params.ProxyPublicURL, + ClusterName: params.ClusterName, + IntegrationName: params.IntegrationName, } return trace.Wrap(awsoidc.ConfigureEC2SSM(ctx, awsClt, confReq)) } diff --git a/tool/teleport/common/teleport.go b/tool/teleport/common/teleport.go index d912e97c68b9d..1517742be9fb6 100644 --- a/tool/teleport/common/teleport.go +++ b/tool/teleport/common/teleport.go @@ -474,6 +474,8 @@ func Run(options Options) (app *kingpin.Application, executedCommand string, con integrationConfEC2SSMCmd.Flag("ssm-document-name", "The AWS SSM Document name to create that will be used to install teleport.").Required().StringVar(&ccf.IntegrationConfEC2SSMIAMArguments.SSMDocumentName) integrationConfEC2SSMCmd.Flag("proxy-public-url", "Proxy Public URL (eg https://mytenant.teleport.sh).").StringVar(&ccf. IntegrationConfEC2SSMIAMArguments.ProxyPublicURL) + integrationConfEC2SSMCmd.Flag("cluster", "Teleport Cluster's name.").Required().StringVar(&ccf.IntegrationConfEC2SSMIAMArguments.ClusterName) + integrationConfEC2SSMCmd.Flag("name", "Integration name.").Required().StringVar(&ccf.IntegrationConfEC2SSMIAMArguments.IntegrationName) integrationConfAWSAppAccessCmd := integrationConfigureCmd.Command("aws-app-access-iam", "Adds required IAM permissions to connect to AWS using App Access.") integrationConfAWSAppAccessCmd.Flag("role", "The AWS Role name used by the AWS OIDC Integration.").Required().StringVar(&ccf.IntegrationConfAWSAppAccessIAMArguments.RoleName) diff --git a/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx b/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx index 83f83ba16a904..cd0e53ac3814f 100644 --- a/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx +++ b/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx @@ -132,6 +132,7 @@ export function DiscoveryConfigSsm() { iamRoleName: arnResourceName, region: selectedRegion, ssmDocument: ssmDocumentName, + integrationName: agentMeta.awsIntegration.name, }); setScriptUrl(scriptUrl); } diff --git a/web/packages/teleport/src/config.ts b/web/packages/teleport/src/config.ts index e48043bdaca26..a45ce6758c381 100644 --- a/web/packages/teleport/src/config.ts +++ b/web/packages/teleport/src/config.ts @@ -322,7 +322,7 @@ const cfg = { '/v1/webapi/scripts/integrations/configure/aws-app-access-iam.sh?role=:iamRoleName', awsConfigureIamEc2AutoDiscoverWithSsmPath: - '/v1/webapi/scripts/integrations/configure/ec2-ssm-iam.sh?role=:iamRoleName&awsRegion=:region&ssmDocument=:ssmDocument', + '/v1/webapi/scripts/integrations/configure/ec2-ssm-iam.sh?role=:iamRoleName&awsRegion=:region&ssmDocument=:ssmDocument&integrationName=:integrationName', eksClustersListPath: '/v1/webapi/sites/:clusterId/integrations/aws-oidc/:name/eksclusters', @@ -1261,6 +1261,7 @@ export interface UrlAwsConfigureIamEc2AutoDiscoverWithSsmScriptParams { region: Regions; iamRoleName: string; ssmDocument: string; + integrationName: string; } export interface UrlGcpWorkforceConfigParam {