Skip to content

Commit

Permalink
Add ownership tags to AWS OIDC EKS Enrollment (#44736) (#45726)
Browse files Browse the repository at this point in the history
* Add ownership tags to AWS OIDC EKS Enrollment

This PR adds the ownership tags to the temporary EKS Access Entry
created during the EKS Enrollment.

There might happen that the user already completed the set up once, and
so they won't be asked to run the configuration script (using oneoff +
cloudshell) again. If there's a access denied, the Access Entry is
created without the tags.

With or without the tags, the Access Entry is removed after the
enrollment process.

* log message asking users to add eks:TagResource action

* fix indentation in docs

* fix new tests
  • Loading branch information
marcoandredinis authored Aug 26, 2024
1 parent 5760e59 commit 6ff03f1
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 19 deletions.
19 changes: 13 additions & 6 deletions lib/auth/integration/integrationv1/awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,13 +498,20 @@ func (s *AWSOIDCService) EnrollEKSClusters(ctx context.Context, req *integration

features := modules.GetModules().Features()

clusterName, err := s.cache.GetClusterName()
if err != nil {
return nil, trace.Wrap(err)
}

enrollmentResponse, err := awsoidc.EnrollEKSClusters(ctx, s.logger, s.clock, publicProxyAddr, credsProvider, enrollEKSClient, awsoidc.EnrollEKSClustersRequest{
Region: req.Region,
ClusterNames: req.GetEksClusterNames(),
EnableAppDiscovery: req.EnableAppDiscovery,
EnableAutoUpgrades: features.AutomaticUpgrades,
IsCloud: features.Cloud,
AgentVersion: req.AgentVersion,
Region: req.Region,
ClusterNames: req.GetEksClusterNames(),
EnableAppDiscovery: req.EnableAppDiscovery,
EnableAutoUpgrades: features.AutomaticUpgrades,
IsCloud: features.Cloud,
AgentVersion: req.AgentVersion,
TeleportClusterName: clusterName.GetClusterName(),
IntegrationName: req.Integration,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down
1 change: 1 addition & 0 deletions lib/cloud/aws/policy_statements.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ func StatementForEKSAccess() *Statement {
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy",
"eks:TagResource",
},
Resources: allResources,
}
Expand Down
48 changes: 44 additions & 4 deletions lib/integrations/awsoidc/eks_enroll_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
apiutils "github.com/gravitational/teleport/api/utils"
awslib "github.com/gravitational/teleport/lib/cloud/aws"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/integrations/awsoidc/tags"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/srv/discovery/common"
"github.com/gravitational/teleport/lib/utils"
Expand Down Expand Up @@ -224,6 +225,14 @@ type EnrollEKSClustersRequest struct {
// ClusterNames is name of the EKS cluster to enroll.
ClusterNames []string

// TeleportClusterName is the name of the Teleport cluster.
// Used to tag resources created during enrollment.
TeleportClusterName string

// IntegrationName is the name of the integration.
// Used to tag resources created during enrollment.
IntegrationName string

// EnableAppDiscovery specifies if we should enable Kubernetes App Discovery inside the enrolled EKS cluster.
EnableAppDiscovery bool

Expand Down Expand Up @@ -251,6 +260,14 @@ func (e *EnrollEKSClustersRequest) CheckAndSetDefaults() error {
return trace.BadParameter("agent version is required")
}

if e.TeleportClusterName == "" {
return trace.BadParameter("teleport cluster name is required")
}

if e.IntegrationName == "" {
return trace.BadParameter("integration name is required")
}

return nil
}

Expand Down Expand Up @@ -322,7 +339,9 @@ func enrollEKSCluster(ctx context.Context, log *slog.Logger, clock clockwork.Clo
return "", trace.Wrap(err)
}

wasAdded, err := maybeAddAccessEntry(ctx, clusterName, principalArn, clt)
ownershipTags := tags.DefaultResourceCreationTags(req.TeleportClusterName, req.IntegrationName)

wasAdded, err := maybeAddAccessEntry(ctx, log, clusterName, principalArn, clt, ownershipTags)
if err != nil {
return "", trace.Wrap(err)
}
Expand Down Expand Up @@ -399,7 +418,7 @@ func getAccessEntryPrincipalArn(ctx context.Context, identityGetter IdentityGett

// maybeAddAccessEntry checks list of access entries for the EKS cluster and adds one for Teleport if it's missing.
// If access entry was added by this function it will return true as a first value.
func maybeAddAccessEntry(ctx context.Context, clusterName, roleArn string, clt EnrollEKSCLusterClient) (bool, error) {
func maybeAddAccessEntry(ctx context.Context, log *slog.Logger, clusterName, roleArn string, clt EnrollEKSCLusterClient, ownershipTags tags.AWSTags) (bool, error) {
entries, err := clt.ListAccessEntries(ctx, &eks.ListAccessEntriesInput{
ClusterName: aws.String(clusterName),
})
Expand All @@ -413,10 +432,31 @@ func maybeAddAccessEntry(ctx context.Context, clusterName, roleArn string, clt E
}
}

_, err = clt.CreateAccessEntry(ctx, &eks.CreateAccessEntryInput{
createAccessEntryReq := &eks.CreateAccessEntryInput{
ClusterName: aws.String(clusterName),
PrincipalArn: aws.String(roleArn),
})
Tags: ownershipTags.ToMap(),
}

_, err = clt.CreateAccessEntry(ctx, createAccessEntryReq)
if err != nil {
convertedError := awslib.ConvertIAMv2Error(err)
if !trace.IsAccessDenied(convertedError) {
return false, trace.Wrap(err)
}
// Adding tags requires the `eks:TagResource` action.
// This action is now part of the added policies, for previous set ups we didn't include the tag resource action in the policy document.
// See lib/cloud/aws.StatementForEKSAccess
// Instead of failing with an error, the Access Entry is created anyway without tags.
// This resource is meant to be deleted right after the teleport agent is installed.
createAccessEntryReq.Tags = nil

log.WarnContext(ctx, "Failed to tag EKS Access Entry, please add eks:TagResource action in IAM Role. Continuing without tags.",
"principal", roleArn,
"cluster", clusterName,
)
_, err = clt.CreateAccessEntry(ctx, createAccessEntryReq)
}
return err == nil, trace.Wrap(err)
}

Expand Down
30 changes: 22 additions & 8 deletions lib/integrations/awsoidc/eks_enroll_clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"fmt"
"log/slog"
"maps"
"slices"
"strings"
"testing"
Expand Down Expand Up @@ -137,9 +138,11 @@ func TestEnrollEKSClusters(t *testing.T) {
return clt
}
baseRequest := EnrollEKSClustersRequest{
Region: "us-east-1",
AgentVersion: "1.2.3",
EnableAppDiscovery: true,
Region: "us-east-1",
AgentVersion: "1.2.3",
EnableAppDiscovery: true,
IntegrationName: "my-integration",
TeleportClusterName: "my-teleport-cluster",
}

clock := clockwork.NewFakeClockAt(time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC))
Expand Down Expand Up @@ -251,9 +254,11 @@ func TestEnrollEKSClusters(t *testing.T) {
},
},
request: EnrollEKSClustersRequest{
Region: "us-east-1",
AgentVersion: "1.2.3",
IsCloud: true,
Region: "us-east-1",
AgentVersion: "1.2.3",
IsCloud: true,
IntegrationName: "my-integration",
TeleportClusterName: "my-teleport-cluster",
},
requestClusterNames: []string{"EKS3"},
responseCheck: func(t *testing.T, response *EnrollEKSClusterResponse) {
Expand All @@ -278,8 +283,10 @@ func TestEnrollEKSClusters(t *testing.T) {
},
},
request: EnrollEKSClustersRequest{
Region: "us-east-1",
AgentVersion: "1.2.3",
Region: "us-east-1",
AgentVersion: "1.2.3",
IntegrationName: "my-integration",
TeleportClusterName: "my-teleport-cluster",
},
requestClusterNames: []string{"EKS3"},
responseCheck: func(t *testing.T, response *EnrollEKSClusterResponse) {
Expand Down Expand Up @@ -360,8 +367,10 @@ func TestEnrollEKSClusters(t *testing.T) {
mockClt, ok := client.(*mockEnrollEKSClusterClient)
require.True(t, ok)
createCalled, deleteCalled := false, false
createTags := make(map[string]string)
mockClt.createAccessEntry = func(ctx context.Context, input *eks.CreateAccessEntryInput, f ...func(*eks.Options)) (*eks.CreateAccessEntryOutput, error) {
createCalled = true
createTags = maps.Clone(input.Tags)
return nil, nil
}
mockClt.deleteAccessEntry = func(ctx context.Context, input *eks.DeleteAccessEntryInput, f ...func(*eks.Options)) (*eks.DeleteAccessEntryOutput, error) {
Expand All @@ -375,6 +384,11 @@ func TestEnrollEKSClusters(t *testing.T) {
require.Len(t, response.Results, 1)
require.Equal(t, "EKS1", response.Results[0].ClusterName)
require.True(t, createCalled)
require.Equal(t, map[string]string{
"teleport.dev/cluster": "my-teleport-cluster",
"teleport.dev/integration": "my-integration",
"teleport.dev/origin": "integration_awsoidc",
}, createTags)
require.True(t, deleteCalled)
})
}
Expand Down
1 change: 1 addition & 0 deletions lib/integrations/awsoidc/eks_iam_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func NewEKSIAMConfigureClient(ctx context.Context, region string) (EKSIAMConfigu
// - eks:CreateAccessEntry
// - eks:DeleteAccessEntry
// - eks:AssociateAccessPolicy
// - eks:TagResource
//
// For more info about EKS access entries see:
// https://aws.amazon.com/blogs/containers/a-deep-dive-into-simplified-amazon-eks-access-management-controls/
Expand Down
3 changes: 3 additions & 0 deletions lib/srv/discovery/fetchers/eks.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ func (a *eksFetcher) checkOrSetupAccessForARN(ctx context.Context, client eksifa
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy",
"eks:TagResource",
})
return nil
case err == nil:
Expand All @@ -438,6 +439,7 @@ func (a *eksFetcher) checkOrSetupAccessForARN(ctx context.Context, client eksifa
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy",
"eks:TagResource",
})
return nil
} else if err != nil {
Expand All @@ -457,6 +459,7 @@ func (a *eksFetcher) checkOrSetupAccessForARN(ctx context.Context, client eksifa
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy",
"eks:TagResource",
})
return nil
}
Expand Down
3 changes: 2 additions & 1 deletion rfd/0157-aws-eks-discover.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ associate access policies for an EKS cluster (details [below](#eks-access-entrie
"eks:ListAccessEntries",
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy"
"eks:AssociateAccessPolicy",
"eks:TagResource"
],
"Resource": "*"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function ConfigureIamPerms({
"eks:CreateAccessEntry",
"eks:DeleteAccessEntry",
"eks:AssociateAccessPolicy",
"eks:TagResource"
],
"Resource": "*"
}
Expand Down

0 comments on commit 6ff03f1

Please sign in to comment.