From 64b8397bb5e1eceac1eaabc1643ca6aa270224ac Mon Sep 17 00:00:00 2001 From: Alex Hemard Date: Thu, 25 Sep 2025 13:45:40 -0500 Subject: [PATCH] feat(AWS OIDC): deleting integration removes associated resources --- api/types/discoveryconfig/discoveryconfig.go | 9 + .../discoveryconfig/discoveryconfig_test.go | 107 ++++++++++++ lib/auth/authclient/api.go | 3 + lib/auth/integration/integrationv1/awsoidc.go | 98 +++++++++++ lib/auth/integration/integrationv1/service.go | 10 ++ .../integration/integrationv1/service_test.go | 161 ++++++++++++++++++ .../DeleteAwsOidcIntegrationDialog.tsx | 100 +++++++++++ .../src/Integrations/Integrations.story.tsx | 21 +++ .../Operations/IntegrationOperations.tsx | 11 ++ .../Operations/useIntegrationOperation.ts | 2 +- .../status/AwsOidc/AwsOidcTitle.tsx | 5 +- 11 files changed, 524 insertions(+), 3 deletions(-) create mode 100644 web/packages/teleport/src/Integrations/DeleteAwsOidcIntegrationDialog.tsx diff --git a/api/types/discoveryconfig/discoveryconfig.go b/api/types/discoveryconfig/discoveryconfig.go index 64d42367e773f..a2cd66eff97b0 100644 --- a/api/types/discoveryconfig/discoveryconfig.go +++ b/api/types/discoveryconfig/discoveryconfig.go @@ -201,6 +201,15 @@ func (a *DiscoveryConfig) MatchSearch(values []string) bool { return types.MatchSearch(fieldVals, values, nil) } +// IsMatchersEmpty returns true if all matchers are empty. +func (a *DiscoveryConfig) IsMatchersEmpty() bool { + return len(a.Spec.AWS) == 0 && + len(a.Spec.Azure) == 0 && + len(a.Spec.GCP) == 0 && + len(a.Spec.Kube) == 0 && + (a.Spec.AccessGraph == nil || len(a.Spec.AccessGraph.AWS) == 0) +} + // CloneResource returns a copy of the resource as types.ResourceWithLabels. func (a *DiscoveryConfig) CloneResource() types.ResourceWithLabels { var copy *DiscoveryConfig diff --git a/api/types/discoveryconfig/discoveryconfig_test.go b/api/types/discoveryconfig/discoveryconfig_test.go index 39c2cca20643b..b8439dd0aa015 100644 --- a/api/types/discoveryconfig/discoveryconfig_test.go +++ b/api/types/discoveryconfig/discoveryconfig_test.go @@ -451,3 +451,110 @@ func TestNewDiscoveryConfig(t *testing.T) { }) } } + +func TestDiscoveryConfig_IsMatchersEmpty(t *testing.T) { + for _, tt := range []struct { + name string + config *DiscoveryConfig + expected bool + }{ + { + name: "empty config", + config: &DiscoveryConfig{ + Spec: Spec{}, + }, + expected: true, + }, + { + name: "has AWS matchers", + config: &DiscoveryConfig{ + Spec: Spec{ + AWS: []types.AWSMatcher{{ + Types: []string{"ec2"}, + Regions: []string{"us-west-2"}, + }}, + }, + }, + expected: false, + }, + { + name: "has Azure matchers", + config: &DiscoveryConfig{ + Spec: Spec{ + Azure: []types.AzureMatcher{{ + Types: []string{"vm"}, + Regions: []string{"europe-west-2"}, + }}, + }, + }, + expected: false, + }, + { + name: "has GCP matchers", + config: &DiscoveryConfig{ + Spec: Spec{ + GCP: []types.GCPMatcher{{ + Types: []string{"gce"}, + ProjectIDs: []string{"p1"}, + }}, + }, + }, + expected: false, + }, + { + name: "has Kube matchers", + config: &DiscoveryConfig{ + Spec: Spec{ + Kube: []types.KubernetesMatcher{{ + Types: []string{"app"}, + }}, + }, + }, + expected: false, + }, + { + name: "has AccessGraph with AWS", + config: &DiscoveryConfig{ + Spec: Spec{ + AccessGraph: &types.AccessGraphSync{ + AWS: []*types.AccessGraphAWSSync{{ + Integration: "integration1", + Regions: []string{"us-west-2"}, + }}, + }, + }, + }, + expected: false, + }, + { + name: "has AccessGraph but no AWS", + config: &DiscoveryConfig{ + Spec: Spec{ + AccessGraph: &types.AccessGraphSync{}, + }, + }, + expected: true, + }, + { + name: "has multiple matcher types", + config: &DiscoveryConfig{ + Spec: Spec{ + AWS: []types.AWSMatcher{{ + Types: []string{"ec2"}, + Regions: []string{"us-west-2"}, + }}, + Azure: []types.AzureMatcher{{ + Types: []string{"vm"}, + Regions: []string{"europe-west-2"}, + }}, + }, + }, + expected: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + got := tt.config.IsMatchersEmpty() + require.Equal(t, tt.expected, got) + }) + } +} diff --git a/lib/auth/authclient/api.go b/lib/auth/authclient/api.go index 2ee2610ab4926..ce280b502ccc8 100644 --- a/lib/auth/authclient/api.go +++ b/lib/auth/authclient/api.go @@ -1301,6 +1301,9 @@ type Cache interface { // UserLoginStatesGetter defines methods for fetching user login states. services.UserLoginStatesGetter + + // DiscoveryConfigsGetter defines methods for fetching discovery configs. + services.DiscoveryConfigsGetter } type NodeWrapper struct { diff --git a/lib/auth/integration/integrationv1/awsoidc.go b/lib/auth/integration/integrationv1/awsoidc.go index 75d94d6e2474f..62e68d707057e 100644 --- a/lib/auth/integration/integrationv1/awsoidc.go +++ b/lib/auth/integration/integrationv1/awsoidc.go @@ -21,14 +21,19 @@ package integrationv1 import ( "context" "log/slog" + "slices" + "strings" + "github.com/google/uuid" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/defaults" integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1" "github.com/gravitational/teleport/api/types" awsutils "github.com/gravitational/teleport/api/utils/aws" + "github.com/gravitational/teleport/api/utils/clientutils" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/integrations/awsoidc" "github.com/gravitational/teleport/lib/modules" @@ -85,6 +90,99 @@ type AWSOIDCServiceConfig struct { Logger *slog.Logger } +// deleteAWSOIDCAssociatedResources deletes associated discovery_configs and +// app_servers created by the integration being deleted. Should only be used by +// methods that check access for VerbDelete on KindIntegration +func (s *Service) deleteAWSOIDCAssociatedResources(ctx context.Context, authCtx *authz.Context, ig types.Integration) error { + // TODO(alexhemard): follow up work needed to add explicit labels for + // resources created by integration rather than rely on implicit rules + + // Delete discovery_configs created by this integration + var configsRequireCleanup []string + var configsToDelete []string + + for config, err := range clientutils.Resources(ctx, s.cache.ListDiscoveryConfigs) { + if err != nil { + return trace.Wrap(err) + } + + awsMatchers := config.Spec.AWS + + config.Spec.AWS = slices.DeleteFunc(config.Spec.AWS, func(matcher types.AWSMatcher) bool { + return matcher.Integration == ig.GetName() + }) + + if len(awsMatchers) == len(config.Spec.AWS) { + continue + } + + // discovery_configs can be assumed to be created by the integration + // and deleted if + // 1. only has matchers referencing this integration + // 2. has valid uuid name + if config.IsMatchersEmpty() { + _, err := uuid.Parse(config.GetName()) + + if err == nil { + configsToDelete = append(configsToDelete, config.GetName()) + continue + } + } + + configsRequireCleanup = append(configsRequireCleanup, config.GetName()) + } + + if len(configsRequireCleanup) > 0 { + var qualifiedConfigs []string + for _, config := range configsRequireCleanup { + qualifiedConfigs = append(qualifiedConfigs, "discovery_config/"+config) + } + + return trace.BadParameter("cannot delete integration, "+ + "Discovery Configs referencing this integration must be removed first: %s\n\n"+ + "Use `tsh rm %s` to remove them.", + strings.Join(configsRequireCleanup, ", "), + strings.Join(qualifiedConfigs, " ")) + } + + for _, configName := range configsToDelete { + s.logger.DebugContext(ctx, "Deleting discovery_config associated with integration", + "discovery_config", configName, + "integration", ig.GetName()) + + err := s.backend.DeleteDiscoveryConfig(ctx, configName) + + if err != nil && !trace.IsNotFound(err) { + return trace.Wrap(err) + } + } + + // Delete AWS access app_server associated with this integration + appServers, err := s.cache.GetApplicationServers(ctx, defaults.Namespace) + if err != nil { + return trace.Wrap(err) + } + + for _, appServer := range appServers { + if appServer.GetApp().GetIntegration() == ig.GetName() { + s.logger.DebugContext(ctx, "Deleting app_server associated with integration", + "app_server", appServer.GetName(), + "integration", ig.GetName()) + + err := s.backend.DeleteApplicationServer(ctx, + appServer.GetNamespace(), appServer.GetHostID(), appServer.GetName()) + + if err != nil && !trace.IsNotFound(err) { + return trace.Wrap(err) + } + + break + } + } + + return nil +} + // CheckAndSetDefaults checks the AWSOIDCServiceConfig fields and returns an error if a required param is not provided. // Authorizer and IntegrationService are required params. func (s *AWSOIDCServiceConfig) CheckAndSetDefaults() error { diff --git a/lib/auth/integration/integrationv1/service.go b/lib/auth/integration/integrationv1/service.go index e2ee691cf008f..9d0ee57088315 100644 --- a/lib/auth/integration/integrationv1/service.go +++ b/lib/auth/integration/integrationv1/service.go @@ -59,6 +59,12 @@ type Cache interface { // IntegrationsGetter defines methods to access Integration resources. services.IntegrationsGetter + // DiscoveryConfigsGetter defines methods to access DiscoveryConfig resources. + services.DiscoveryConfigsGetter + + // AppServersGetter defines methods to access application servers. + services.AppServersGetter + // GetPluginStaticCredentialsByLabels will get a list of plugin static credentials resource by matching labels. GetPluginStaticCredentialsByLabels(ctx context.Context, labels map[string]string) ([]types.PluginStaticCredentials, error) } @@ -82,6 +88,8 @@ type Backend interface { services.Integrations services.PluginStaticCredentials services.GitServers + services.DiscoveryConfigs + services.Presence } // ServiceConfig holds configuration options for @@ -495,6 +503,8 @@ func (s *Service) ensureNoGitHubAssociatedResources(ctx context.Context, ig type func (s *Service) deleteAssociatedResources(ctx context.Context, authCtx *authz.Context, ig types.Integration) error { switch ig.GetSubKind() { + case types.IntegrationSubKindAWSOIDC: + return trace.Wrap(s.deleteAWSOIDCAssociatedResources(ctx, authCtx, ig)) case types.IntegrationSubKindGitHub: return trace.Wrap(s.deleteGitHubAssociatedResources(ctx, authCtx, ig)) default: diff --git a/lib/auth/integration/integrationv1/service_test.go b/lib/auth/integration/integrationv1/service_test.go index 7597e7efec399..cc83a3db1eb1d 100644 --- a/lib/auth/integration/integrationv1/service_test.go +++ b/lib/auth/integration/integrationv1/service_test.go @@ -31,9 +31,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/api/defaults" integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/types/discoveryconfig" "github.com/gravitational/teleport/api/types/externalauditstorage" + "github.com/gravitational/teleport/api/types/header" "github.com/gravitational/teleport/lib/auth/integration/credentials" "github.com/gravitational/teleport/lib/auth/keystore" "github.com/gravitational/teleport/lib/auth/testauthority" @@ -666,6 +669,108 @@ func TestIntegrationCRUD(t *testing.T) { }, ErrAssertion: noError, }, + { + Name: "cannot delete AWS OIDC integration with user-created discovery config", + Role: types.RoleSpecV6{ + Allow: types.RoleConditions{Rules: []types.Rule{ + { + Resources: []string{types.KindIntegration}, + Verbs: []string{types.VerbDelete}, + }, + }}, + }, + Setup: func(t *testing.T, igName string) { + t.Helper() + ig := sampleIntegrationFn(t, igName) + _, err := localClient.CreateIntegration(ctx, ig) + require.NoError(t, err) + _, err = localClient.CreateDiscoveryConfig(ctx, mustMakeDiscoveryConfig(t, ig)) + require.NoError(t, err) + problematicConfig := mustMakeDiscoveryConfig(t, ig) + problematicConfig.Metadata.Name = "problematicconfig" + _, err = localClient.CreateDiscoveryConfig(ctx, problematicConfig) + require.NoError(t, err) + }, + Test: func(ctx context.Context, resourceSvc *Service, igName string) error { + _, err := resourceSvc.DeleteIntegration(ctx, &integrationpb.DeleteIntegrationRequest{ + Name: igName, + DeleteAssociatedResources: true, + }) + return err + }, + Validate: func(t *testing.T, igName string) { + t.Helper() + _, err := localClient.GetIntegration(context.Background(), igName) + require.NoError(t, err) + _, err = localClient.GetDiscoveryConfig(context.Background(), igName) + require.NoError(t, err) + _, err = localClient.GetDiscoveryConfig(context.Background(), "problematicconfig") + require.NoError(t, err) + }, + ErrAssertion: trace.IsBadParameter, + }, + { + Name: "delete AWS OIDC integration with associated resources", + Role: types.RoleSpecV6{ + Allow: types.RoleConditions{Rules: []types.Rule{ + { + Resources: []string{types.KindIntegration}, + Verbs: []string{types.VerbDelete}, + }, + }}, + }, + Setup: func(t *testing.T, igName string) { + t.Helper() + + ig := sampleIntegrationFn(t, igName) + _, err := localClient.CreateIntegration(ctx, ig) + require.NoError(t, err) + _, err = localClient.CreateDiscoveryConfig(ctx, mustMakeDiscoveryConfig(t, ig)) + require.NoError(t, err) + _, err = localClient.UpsertApplicationServer(ctx, mustMakeAppServer(t, ig)) + require.NoError(t, err) + + anotherIg := sampleIntegrationFn(t, igName+igName) + _, err = localClient.CreateIntegration(ctx, anotherIg) + require.NoError(t, err) + _, err = localClient.CreateDiscoveryConfig(ctx, mustMakeDiscoveryConfig(t, anotherIg)) + require.NoError(t, err) + _, err = localClient.UpsertApplicationServer(ctx, mustMakeAppServer(t, anotherIg)) + require.NoError(t, err) + }, + Test: func(ctx context.Context, resourceSvc *Service, igName string) error { + _, err := resourceSvc.DeleteIntegration(ctx, &integrationpb.DeleteIntegrationRequest{ + Name: igName, + DeleteAssociatedResources: true, + }) + return err + }, + Validate: func(t *testing.T, igName string) { + t.Helper() + // discovery_config associated with the integration is removed + _, err := localClient.GetDiscoveryConfig(context.Background(), igName) + require.True(t, trace.IsNotFound(err)) + // app_server associated with the integration is removed + appServers, err := localClient.GetApplicationServers(context.Background(), defaults.Namespace) + require.NoError(t, err) + for _, appServer := range appServers { + require.NotEqual(t, igName, appServer.GetApp().GetIntegration(), + "app server with integration %s should have been deleted", igName) + } + // other integrations' associated resources should remain + _, err = localClient.GetDiscoveryConfig(context.Background(), igName+igName) + require.NoError(t, err) + require.Condition(t, func() bool { + for _, appServer := range appServers { + if appServer.GetApp().GetIntegration() == igName+igName { + return true + } + } + return false + }, "app server with integration %s should still exist", igName+igName) + }, + ErrAssertion: noError, + }, // Delete all { @@ -746,6 +851,8 @@ type localClient interface { services.PluginStaticCredentials services.Integrations services.GitServers + services.DiscoveryConfigs + services.Presence } type testClient struct { @@ -854,6 +961,8 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl require.NoError(t, err) gitServerService, err := local.NewGitServerService(backend) require.NoError(t, err) + discoveryConfigService, err := local.NewDiscoveryConfigService(backend) + require.NoError(t, err) authorizer, err := authz.NewAuthorizer(authz.AuthorizerOpts{ ClusterName: clusterName, @@ -866,15 +975,20 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl require.NoError(t, err) localCredService, err := local.NewPluginStaticCredentialsService(backend) require.NoError(t, err) + presenceService := local.NewPresenceService(backend) backendSvc := struct { services.Integrations services.PluginStaticCredentials services.GitServers + services.DiscoveryConfigs + services.Presence }{ Integrations: localResourceService, PluginStaticCredentials: localCredService, GitServers: gitServerService, + DiscoveryConfigs: discoveryConfigService, + Presence: presenceService, } cacheResourceService, err := local.NewIntegrationsService(backend, local.WithIntegrationsServiceCacheMode(true)) @@ -890,6 +1004,8 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl }, IntegrationsService: *cacheResourceService, PluginStaticCredentialsService: localCredService, + DiscoveryConfigService: discoveryConfigService, + PresenceService: presenceService, } keystoreManager, err := keystore.NewManager(t.Context(), &servicecfg.KeystoreConfig{}, &keystore.Options{ @@ -924,6 +1040,8 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl *local.PluginsService *local.PluginStaticCredentialsService *local.GitServerService + *local.DiscoveryConfigService + *local.PresenceService }{ AccessService: roleSvc, IdentityService: userSvc, @@ -932,6 +1050,8 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl PluginsService: pluginSvc, PluginStaticCredentialsService: localCredService, GitServerService: gitServerService, + DiscoveryConfigService: discoveryConfigService, + PresenceService: presenceService, }, resourceSvc } @@ -944,6 +1064,8 @@ type mockCache struct { local.IntegrationsService *local.PluginStaticCredentialsService + *local.DiscoveryConfigService + *local.PresenceService } func (m *mockCache) GetProxies() ([]types.Server, error) { @@ -1073,3 +1195,42 @@ func mustMakeGitHubServer(t *testing.T, ig types.Integration) types.Server { server.SetName(ig.GetName()) return server } + +func mustMakeDiscoveryConfig(t *testing.T, ig types.Integration) *discoveryconfig.DiscoveryConfig { + t.Helper() + discoveryConfig, err := discoveryconfig.NewDiscoveryConfig( + header.Metadata{Name: ig.GetName()}, + discoveryconfig.Spec{ + DiscoveryGroup: ig.GetName(), + AWS: []types.AWSMatcher{ + { + Types: []string{"ec2"}, + Regions: []string{"us-west-2"}, + Integration: ig.GetName(), + }, + }, + }, + ) + require.NoError(t, err) + return discoveryConfig +} + +func mustMakeAppServer(t *testing.T, ig types.Integration) types.AppServer { + t.Helper() + appServer, err := types.NewAppServerV3(types.Metadata{ + Name: ig.GetName(), + }, types.AppServerSpecV3{ + HostID: ig.GetName(), + App: &types.AppV3{ + Metadata: types.Metadata{ + Name: ig.GetName(), + }, + Spec: types.AppSpecV3{ + URI: "https://example.com", + Integration: ig.GetName(), + }, + }, + }) + require.NoError(t, err) + return appServer +} diff --git a/web/packages/teleport/src/Integrations/DeleteAwsOidcIntegrationDialog.tsx b/web/packages/teleport/src/Integrations/DeleteAwsOidcIntegrationDialog.tsx new file mode 100644 index 0000000000000..4fe37c8cefe8e --- /dev/null +++ b/web/packages/teleport/src/Integrations/DeleteAwsOidcIntegrationDialog.tsx @@ -0,0 +1,100 @@ +/** + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { Alert, ButtonSecondary, ButtonWarning, P1, Text } from 'design'; +import Dialog, { + DialogContent, + DialogFooter, + DialogHeader, + DialogTitle, +} from 'design/DialogConfirmation'; +import useAttempt from 'shared/hooks/useAttemptNext'; + +import { IntegrationAwsOidc } from 'teleport/services/integrations'; +import useStickyClusterId from 'teleport/useStickyClusterId'; + +import { type DeleteRequestOptions } from './Operations/IntegrationOperations'; + +type Props = { + close(): void; + remove(opt?: DeleteRequestOptions): Promise; + integration: IntegrationAwsOidc; +}; + +export function DeleteAwsOidcIntegrationDialog(props: Props) { + const { close, remove, integration } = props; + const { attempt, run } = useAttempt(); + const isDisabled = attempt.status === 'processing'; + const { clusterId } = useStickyClusterId(); + + // https://resource-explorer.console.aws.amazon.com + // /resource-explorer/home#/search?query= + // tag:teleport.dev/origin=integration_awsoidc+ + // tag:teleport.dev/cluster={cluster_name}+ + // tag:teleport.dev/integration={integration_name} + const awsResourceExplorerUrl = + `https://resource-explorer.console.aws.amazon.com` + + `/resource-explorer/home#/search?query=` + + `tag%3Ateleport.dev%2Forigin%3Dintegration_awsoidc+` + + `tag%3Ateleport.dev%2Fcluster%3D${clusterId}+` + + `tag%3Ateleport.dev%2Fintegration%3D${integration.name}`; + function onOk() { + run(() => remove({ deleteAssociatedResources: true })); + } + + return ( + + + Delete AWS OIDC Integration? + + + {attempt.status === 'failed' && } + + Are you sure you want to delete integration{' '} + + {integration.name} + + ? + + + AWS resources created by this integration may require manual clean up. + Visit AWS Resource Explorer to see AWS resources tagged by this + integration. + + + Teleport will also remove App servers and resources used for + auto-discovery that reference this integration. + + + + + Yes, Delete Integration + + + Cancel + + + + ); +} diff --git a/web/packages/teleport/src/Integrations/Integrations.story.tsx b/web/packages/teleport/src/Integrations/Integrations.story.tsx index 32cd1fa5fcb88..7c0adcbc64191 100644 --- a/web/packages/teleport/src/Integrations/Integrations.story.tsx +++ b/web/packages/teleport/src/Integrations/Integrations.story.tsx @@ -23,6 +23,7 @@ import { IntegrationStatusCode, } from 'teleport/services/integrations'; +import { DeleteAwsOidcIntegrationDialog } from './DeleteAwsOidcIntegrationDialog'; import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog'; import { integrations, plugins } from './fixtures'; import { IntegrationList } from './IntegrationList'; @@ -50,6 +51,26 @@ export function DeleteDialog() { ); } +export function DeleteAwsOidcDialog() { + return ( + + null} + remove={() => null} + integration={{ + resourceType: 'integration', + kind: IntegrationKind.AwsOidc, + name: 'some-integration-name', + spec: { + roleArn: 'arn:aws:iam::123456789012:role/johndoe', + }, + statusCode: IntegrationStatusCode.Running, + }} + /> + + ); +} + export function EditDialogWithoutS3() { return ( ) { + if (operation === 'delete' && integration.kind === IntegrationKind.AwsOidc) { + return ( + + ); + } + if (operation === 'delete') { return (