From fefd9e0babb60a1585074efa1e6b1b3b4070dd9f Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Jan 2025 12:06:02 +0000 Subject: [PATCH 1/5] Add "unstable" disable join attrs flag --- lib/tlsca/ca.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index a7e6ad24e39e4..9820ca198b520 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -31,6 +31,7 @@ import ( "fmt" "math/big" "net" + "os" "strconv" "time" @@ -905,7 +906,7 @@ func (id *Identity) Subject() (pkix.Name, error) { ) } - if id.JoinAttributes != nil { + if id.JoinAttributes != nil && shouldPersistJoinAttrs() { encoded, err := protojson.MarshalOptions{ // Use the proto field names as this is what we use in the // templating engine and this being consistent for any user who @@ -1353,3 +1354,10 @@ func (ca *CertAuthority) GenerateCertificate(req CertificateRequest) ([]byte, er return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes}), nil } + +// shouldPersistJoinAttrs returns true if the join attributes should be persisted +// into the X509 identity. This provides an emergency "off" handle for this +// new behaviour until we are confident it is working as expected. +func shouldPersistJoinAttrs() bool { + return os.Getenv("TELEPORT_UNSTABLE_DISABLE_JOIN_ATTRS") != "1" +} From acc90264fed9b2852ba7aac26edc24871c662eaa Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Jan 2025 12:12:53 +0000 Subject: [PATCH 2/5] Remove experimental ff --- .../experiment/experiment.go | 41 ------------------- .../workloadidentityv1/issuer_service.go | 9 ---- .../workloadidentityv1_test.go | 13 ++---- .../service_workload_identity_api_test.go | 5 +-- .../service_workload_identity_jwt_test.go | 5 +-- .../service_workload_identity_x509_test.go | 5 +-- 6 files changed, 6 insertions(+), 72 deletions(-) delete mode 100644 lib/auth/machineid/workloadidentityv1/experiment/experiment.go diff --git a/lib/auth/machineid/workloadidentityv1/experiment/experiment.go b/lib/auth/machineid/workloadidentityv1/experiment/experiment.go deleted file mode 100644 index fafe51ea83d1f..0000000000000 --- a/lib/auth/machineid/workloadidentityv1/experiment/experiment.go +++ /dev/null @@ -1,41 +0,0 @@ -// Teleport -// Copyright (C) 2024 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 . - -package experiment - -import ( - "os" - "sync" -) - -var mu sync.Mutex - -var experimentEnabled = os.Getenv("TELEPORT_WORKLOAD_IDENTITY_UX_EXPERIMENT") == "1" - -// Enabled returns true if the workload identity UX experiment is -// enabled. -func Enabled() bool { - mu.Lock() - defer mu.Unlock() - return experimentEnabled -} - -// SetEnabled sets the experiment enabled flag. -func SetEnabled(enabled bool) { - mu.Lock() - defer mu.Unlock() - experimentEnabled = enabled -} diff --git a/lib/auth/machineid/workloadidentityv1/issuer_service.go b/lib/auth/machineid/workloadidentityv1/issuer_service.go index 6842ae01632ec..73694e0de54e8 100644 --- a/lib/auth/machineid/workloadidentityv1/issuer_service.go +++ b/lib/auth/machineid/workloadidentityv1/issuer_service.go @@ -39,7 +39,6 @@ import ( "github.com/gravitational/teleport/api/observability/tracing" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" - "github.com/gravitational/teleport/lib/auth/machineid/workloadidentityv1/experiment" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/jwt" @@ -147,10 +146,6 @@ func (s *IssuanceService) IssueWorkloadIdentity( ctx context.Context, req *workloadidentityv1pb.IssueWorkloadIdentityRequest, ) (*workloadidentityv1pb.IssueWorkloadIdentityResponse, error) { - if !experiment.Enabled() { - return nil, trace.AccessDenied("workload identity issuance experiment is disabled") - } - switch { case req.GetName() == "": return nil, trace.BadParameter("name: is required") @@ -240,10 +235,6 @@ func (s *IssuanceService) IssueWorkloadIdentities( ctx context.Context, req *workloadidentityv1pb.IssueWorkloadIdentitiesRequest, ) (*workloadidentityv1pb.IssueWorkloadIdentitiesResponse, error) { - if !experiment.Enabled() { - return nil, trace.AccessDenied("workload identity issuance experiment is disabled") - } - switch { case len(req.LabelSelectors) == 0: return nil, trace.BadParameter("label_selectors: at least one label selector must be specified") diff --git a/lib/auth/machineid/workloadidentityv1/workloadidentityv1_test.go b/lib/auth/machineid/workloadidentityv1/workloadidentityv1_test.go index 1ddf63bcf28d1..167ee3f33e1f2 100644 --- a/lib/auth/machineid/workloadidentityv1/workloadidentityv1_test.go +++ b/lib/auth/machineid/workloadidentityv1/workloadidentityv1_test.go @@ -52,7 +52,6 @@ import ( "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/join" - "github.com/gravitational/teleport/lib/auth/machineid/workloadidentityv1/experiment" "github.com/gravitational/teleport/lib/auth/state" "github.com/gravitational/teleport/lib/cryptosuites" libevents "github.com/gravitational/teleport/lib/events" @@ -152,9 +151,7 @@ func newIssuanceTestPack(t *testing.T, ctx context.Context) *issuanceTestPack { // APIs necessary for a bot to join and then issue a workload identity are // functioning correctly. func TestIssueWorkloadIdentityE2E(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() tp := newIssuanceTestPack(t, ctx) @@ -339,9 +336,7 @@ func TestIssueWorkloadIdentityE2E(t *testing.T) { } func TestIssueWorkloadIdentity(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() tp := newIssuanceTestPack(t, ctx) @@ -729,9 +724,7 @@ func TestIssueWorkloadIdentity(t *testing.T) { } func TestIssueWorkloadIdentities(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() tp := newIssuanceTestPack(t, ctx) diff --git a/lib/tbot/service_workload_identity_api_test.go b/lib/tbot/service_workload_identity_api_test.go index 6570b343f5686..921b70f398037 100644 --- a/lib/tbot/service_workload_identity_api_test.go +++ b/lib/tbot/service_workload_identity_api_test.go @@ -35,16 +35,13 @@ import ( workloadidentityv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/workloadidentity/v1" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" - "github.com/gravitational/teleport/lib/auth/machineid/workloadidentityv1/experiment" "github.com/gravitational/teleport/lib/tbot/config" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/tool/teleport/testenv" ) func TestBotWorkloadIdentityAPI(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() log := utils.NewSlogLoggerForTests() diff --git a/lib/tbot/service_workload_identity_jwt_test.go b/lib/tbot/service_workload_identity_jwt_test.go index dacd495ee6959..abb785171dac6 100644 --- a/lib/tbot/service_workload_identity_jwt_test.go +++ b/lib/tbot/service_workload_identity_jwt_test.go @@ -30,16 +30,13 @@ import ( workloadidentityv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/workloadidentity/v1" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" - "github.com/gravitational/teleport/lib/auth/machineid/workloadidentityv1/experiment" "github.com/gravitational/teleport/lib/tbot/config" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/tool/teleport/testenv" ) func TestBotWorkloadIdentityJWT(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() log := utils.NewSlogLoggerForTests() diff --git a/lib/tbot/service_workload_identity_x509_test.go b/lib/tbot/service_workload_identity_x509_test.go index 81d0823cc7f4c..00e4317b7a3eb 100644 --- a/lib/tbot/service_workload_identity_x509_test.go +++ b/lib/tbot/service_workload_identity_x509_test.go @@ -32,16 +32,13 @@ import ( workloadidentityv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/workloadidentity/v1" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" - "github.com/gravitational/teleport/lib/auth/machineid/workloadidentityv1/experiment" "github.com/gravitational/teleport/lib/tbot/config" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/tool/teleport/testenv" ) func TestBotWorkloadIdentityX509(t *testing.T) { - experimentStatus := experiment.Enabled() - defer experiment.SetEnabled(experimentStatus) - experiment.SetEnabled(true) + t.Parallel() ctx := context.Background() log := utils.NewSlogLoggerForTests() From 06b2e44c2c0f57ab18808549383a5bac7f0482c8 Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Jan 2025 12:27:48 +0000 Subject: [PATCH 3/5] Remove CLI hidden flags --- lib/tbot/cli/start_workload_identity_api.go | 2 +- lib/tbot/cli/start_workload_identity_jwt.go | 2 +- lib/tbot/cli/start_workload_identity_x509.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/tbot/cli/start_workload_identity_api.go b/lib/tbot/cli/start_workload_identity_api.go index 9dc52024f106e..351fb0345615d 100644 --- a/lib/tbot/cli/start_workload_identity_api.go +++ b/lib/tbot/cli/start_workload_identity_api.go @@ -52,7 +52,7 @@ func NewWorkloadIdentityAPICommand(parentCmd *kingpin.CmdClause, action MutatorA cmd := parentCmd.Command( "workload-identity-api", fmt.Sprintf("%s tbot with a workload identity API listener. Compatible with the SPIFFE Workload API and Envoy SDS.", mode), - ).Hidden() + ) c := &WorkloadIdentityAPICommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_workload_identity_jwt.go b/lib/tbot/cli/start_workload_identity_jwt.go index 9e8a318a23daa..25d548f21eb5b 100644 --- a/lib/tbot/cli/start_workload_identity_jwt.go +++ b/lib/tbot/cli/start_workload_identity_jwt.go @@ -50,7 +50,7 @@ type WorkloadIdentityJWTCommand struct { // result. func NewWorkloadIdentityJWTCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *WorkloadIdentityJWTCommand { // TODO(noah): Unhide this command when feature flag removed - cmd := parentCmd.Command("workload-identity-jwt", fmt.Sprintf("%s tbot with a SPIFFE-compatible JWT SVID output.", mode)).Hidden() + cmd := parentCmd.Command("workload-identity-jwt", fmt.Sprintf("%s tbot with a SPIFFE-compatible JWT SVID output.", mode)) c := &WorkloadIdentityJWTCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_workload_identity_x509.go b/lib/tbot/cli/start_workload_identity_x509.go index 92c322acdc312..c147c6c4aab2d 100644 --- a/lib/tbot/cli/start_workload_identity_x509.go +++ b/lib/tbot/cli/start_workload_identity_x509.go @@ -48,7 +48,7 @@ type WorkloadIdentityX509Command struct { // result. func NewWorkloadIdentityX509Command(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *WorkloadIdentityX509Command { // TODO(noah): Unhide this command when feature flag removed - cmd := parentCmd.Command("workload-identity-x509", fmt.Sprintf("%s tbot with a SPIFFE-compatible SVID output.", mode)).Hidden() + cmd := parentCmd.Command("workload-identity-x509", fmt.Sprintf("%s tbot with a SPIFFE-compatible SVID output.", mode)) c := &WorkloadIdentityX509Command{} c.sharedStartArgs = newSharedStartArgs(cmd) From c392d917a210e319167aa0e1ccabe7254bdf5381 Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Jan 2025 13:45:26 +0000 Subject: [PATCH 4/5] Americanize spelling --- lib/tlsca/ca.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index 9820ca198b520..00b94a9b5e1d2 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -1357,7 +1357,7 @@ func (ca *CertAuthority) GenerateCertificate(req CertificateRequest) ([]byte, er // shouldPersistJoinAttrs returns true if the join attributes should be persisted // into the X509 identity. This provides an emergency "off" handle for this -// new behaviour until we are confident it is working as expected. +// new behavior until we are confident it is working as expected. func shouldPersistJoinAttrs() bool { return os.Getenv("TELEPORT_UNSTABLE_DISABLE_JOIN_ATTRS") != "1" } From 1f35b59c8631c9fd1fd0727c3ff5b503b273f177 Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Tue, 21 Jan 2025 11:41:35 +0000 Subject: [PATCH 5/5] "1" -> "yes" --- lib/tlsca/ca.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index 00b94a9b5e1d2..68c278bdaddb9 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -1359,5 +1359,5 @@ func (ca *CertAuthority) GenerateCertificate(req CertificateRequest) ([]byte, er // into the X509 identity. This provides an emergency "off" handle for this // new behavior until we are confident it is working as expected. func shouldPersistJoinAttrs() bool { - return os.Getenv("TELEPORT_UNSTABLE_DISABLE_JOIN_ATTRS") != "1" + return os.Getenv("TELEPORT_UNSTABLE_DISABLE_JOIN_ATTRS") != "yes" }