Skip to content

Commit

Permalink
Remove forced_rotation feature flag (#5586)
Browse files Browse the repository at this point in the history
Signed-off-by: Agustín Martínez Fayó <[email protected]>
  • Loading branch information
amartinezfayo authored Oct 17, 2024
1 parent 5186212 commit 7abee0a
Show file tree
Hide file tree
Showing 24 changed files with 61 additions and 114 deletions.
91 changes: 30 additions & 61 deletions cmd/spire-server/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package cli
import (
"context"
stdlog "log"
"os"
"strings"

"github.com/mitchellh/cli"
"github.com/spiffe/spire/cmd/spire-server/cli/agent"
Expand All @@ -21,7 +19,6 @@ import (
"github.com/spiffe/spire/cmd/spire-server/cli/upstreamauthority"
"github.com/spiffe/spire/cmd/spire-server/cli/validate"
"github.com/spiffe/spire/cmd/spire-server/cli/x509"
"github.com/spiffe/spire/pkg/common/fflag"
"github.com/spiffe/spire/pkg/common/log"
"github.com/spiffe/spire/pkg/common/version"
)
Expand Down Expand Up @@ -130,75 +127,47 @@ func (cc *CLI) Run(ctx context.Context, args []string) int {
"validate": func() (cli.Command, error) {
return validate.NewValidateCommand(), nil
},
}

// TODO: Remove this when the forced_rotation feature flag is no longer
// needed. Refer to https:/spiffe/spire/issues/5398.
addCommandsEnabledByFFlags(c.Commands)

exitStatus, err := c.Run()
if err != nil {
stdlog.Println(err)
}
return exitStatus
}

// addCommandsEnabledByFFlags adds commands that are currently available only
// through a feature flag.
// Feature flags support through the fflag package in SPIRE Server is
// designed to work only with the run command and the config file.
// Since feature flags are intended to be used by developers of a specific
// feature only, exposing them through command line arguments is not
// convenient. Instead, we use the SPIRE_SERVER_FFLAGS environment variable
// to read the configured SPIRE Server feature flags from the environment
// when other commands may be enabled through feature flags.
func addCommandsEnabledByFFlags(commands map[string]cli.CommandFactory) {
fflagsEnv := os.Getenv("SPIRE_SERVER_FFLAGS")
fflags := strings.Split(fflagsEnv, " ")
flagForcedRotationFound := false
for _, ff := range fflags {
if ff == string(fflag.FlagForcedRotation) {
flagForcedRotationFound = true
break
}
}

if flagForcedRotationFound {
commands["localauthority x509 show"] = func() (cli.Command, error) {
"localauthority x509 show": func() (cli.Command, error) {
return localauthority_x509.NewX509ShowCommand(), nil
}
commands["localauthority x509 prepare"] = func() (cli.Command, error) {
},
"localauthority x509 prepare": func() (cli.Command, error) {
return localauthority_x509.NewX509PrepareCommand(), nil
}
commands["localauthority x509 activate"] = func() (cli.Command, error) {
},
"localauthority x509 activate": func() (cli.Command, error) {
return localauthority_x509.NewX509ActivateCommand(), nil
}
commands["localauthority x509 taint"] = func() (cli.Command, error) {
},
"localauthority x509 taint": func() (cli.Command, error) {
return localauthority_x509.NewX509TaintCommand(), nil
}
commands["localauthority x509 revoke"] = func() (cli.Command, error) {
},
"localauthority x509 revoke": func() (cli.Command, error) {
return localauthority_x509.NewX509RevokeCommand(), nil
}
commands["localauthority jwt show"] = func() (cli.Command, error) {
},
"localauthority jwt show": func() (cli.Command, error) {
return localauthority_jwt.NewJWTShowCommand(), nil
}
commands["localauthority jwt prepare"] = func() (cli.Command, error) {
},
"localauthority jwt prepare": func() (cli.Command, error) {
return localauthority_jwt.NewJWTPrepareCommand(), nil
}
commands["localauthority jwt activate"] = func() (cli.Command, error) {
},
"localauthority jwt activate": func() (cli.Command, error) {
return localauthority_jwt.NewJWTActivateCommand(), nil
}
commands["localauthority jwt taint"] = func() (cli.Command, error) {
},
"localauthority jwt taint": func() (cli.Command, error) {
return localauthority_jwt.NewJWTTaintCommand(), nil
}
commands["localauthority jwt revoke"] = func() (cli.Command, error) {
},
"localauthority jwt revoke": func() (cli.Command, error) {
return localauthority_jwt.NewJWTRevokeCommand(), nil
}
commands["upstreamauthority taint"] = func() (cli.Command, error) {
},
"upstreamauthority taint": func() (cli.Command, error) {
return upstreamauthority.NewTaintCommand(), nil
}
commands["upstreamauthority revoke"] = func() (cli.Command, error) {
},
"upstreamauthority revoke": func() (cli.Command, error) {
return upstreamauthority.NewRevokeCommand(), nil
}
},
}

exitStatus, err := c.Run()
if err != nil {
stdlog.Println(err)
}
return exitStatus
}
8 changes: 1 addition & 7 deletions pkg/common/fflag/fflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ type RawConfig []string
// false, with the only exception being flags that are in the process of being
// deprecated.
const (
// FlagForcedRotation controls whether or not the new APIs and
// extensions related to forced rotation and revocation are
// enabled or not. See #1934 for more information.
FlagForcedRotation Flag = "forced_rotation"

// FlagTestFlag is defined purely for testing purposes.
FlagTestFlag Flag = "i_am_a_test_flag"
)
Expand All @@ -40,8 +35,7 @@ var (
mtx *sync.RWMutex
}{
flags: map[Flag]bool{
FlagForcedRotation: false,
FlagTestFlag: false,
FlagTestFlag: false,
},
loaded: false,
mtx: new(sync.RWMutex),
Expand Down
8 changes: 2 additions & 6 deletions pkg/server/endpoints/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1"
trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1"
"github.com/spiffe/spire/pkg/common/auth"
"github.com/spiffe/spire/pkg/common/fflag"
"github.com/spiffe/spire/pkg/common/peertracker"
"github.com/spiffe/spire/pkg/common/telemetry"
"github.com/spiffe/spire/pkg/common/util"
Expand Down Expand Up @@ -202,11 +201,8 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error {
svidv1.RegisterSVIDServer(udsServer, e.APIServers.SVIDServer)
trustdomainv1.RegisterTrustDomainServer(tcpServer, e.APIServers.TrustDomainServer)
trustdomainv1.RegisterTrustDomainServer(udsServer, e.APIServers.TrustDomainServer)

if fflag.IsSet(fflag.FlagForcedRotation) {
localauthorityv1.RegisterLocalAuthorityServer(tcpServer, e.APIServers.LocalAUthorityServer)
localauthorityv1.RegisterLocalAuthorityServer(udsServer, e.APIServers.LocalAUthorityServer)
}
localauthorityv1.RegisterLocalAuthorityServer(tcpServer, e.APIServers.LocalAUthorityServer)
localauthorityv1.RegisterLocalAuthorityServer(udsServer, e.APIServers.LocalAUthorityServer)

// UDS only
loggerv1.RegisterLoggerServer(udsServer, e.APIServers.LoggerServer)
Expand Down
3 changes: 0 additions & 3 deletions pkg/server/endpoints/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1"
trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
"github.com/spiffe/spire/pkg/common/fflag"
"github.com/spiffe/spire/pkg/common/util"
"github.com/spiffe/spire/pkg/server/authpolicy"
"github.com/spiffe/spire/pkg/server/ca/manager"
Expand Down Expand Up @@ -166,8 +165,6 @@ func TestNewErrorCreatingAuthorizedEntryFetcher(t *testing.T) {
}

func TestListenAndServe(t *testing.T) {
require.NoError(t, fflag.Load(fflag.RawConfig{"forced_rotation"}))

ctx := context.Background()
ca := testca.New(t, testTD)
federatedCA := testca.New(t, foreignFederatedTD)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if [[ $amount_authorities -ne 1 ]]; then
fi

# Prepare authority
prepared_authority_id=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
prepared_authority_id=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server localauthority jwt prepare -output json | jq -r .prepared_authority.authority_id)

# Verify that the prepared authority is logged
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

prepared_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
prepared_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .active.authority_id) || fail-now "Failed to fetch prepared JWT authority ID"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/bin/bash

# Fetch the prepared authority ID
prepared_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
prepared_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .prepared.authority_id) || fail-now "Failed to fetch prepared JWT authority ID"

# Activate the authority
activated_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
activated_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority jwt activate -authorityID "${prepared_authority}" \
-output json | jq -r .activated_authority.authority_id) || fail-now "Failed to activate JWT authority"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ check-logs() {
}

# Fetch old authority ID
old_jwt_authority=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
old_jwt_authority=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .old.authority_id) || fail-now "Failed to fetch old authority ID"

log-debug "Old authority: $old_jwt_authority"

# Taint the old authority
docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
docker compose exec -T spire-server \
/opt/spire/bin/spire-server \
localauthority jwt taint -authorityID "${old_jwt_authority}" || fail-now "Failed to taint old authority"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

active_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
active_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .active.authority_id) || fail-now "Failed to fetch active JWT authority ID"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

old_jwt_authority=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
old_jwt_authority=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .old.authority_id) || fail-now "Failed to fetch old authority ID"

Expand All @@ -22,7 +22,7 @@ if [[ -z "$tainted_found" ]]; then
fail-now "Tainted JWT authority expected"
fi

docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
docker compose exec -T spire-server \
/opt/spire/bin/spire-server localauthority jwt \
revoke -authorityID $old_jwt_authority -output json || fail-now "Failed to revoke JWT authority"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

for i in {1..20}; do
active_jwt_authority=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
active_jwt_authority=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server \
localauthority jwt show -output json | jq -r .active.authority_id) || fail-now "Failed to fetch old jwt authority ID"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ server {
log_level = "DEBUG"
ca_ttl = "24h"
default_jwt_svid_ttl = "8h"
experimental {
feature_flags = ["forced_rotation"]
}
}

plugins {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ for server in intermediateA-server intermediateB-server leafA-server leafB-serve
done

# Prepare authority
prepared_authority_id=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
prepared_authority_id=$(docker compose exec -T root-server \
/opt/spire/bin/spire-server localauthority x509 prepare -output json | jq -r .prepared_authority.authority_id)

# Verify that the prepared authority is logged
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/bin/bash

# Fetch the prepared authority ID
prepared_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
prepared_authority=$(docker compose exec -t root-server \
/opt/spire/bin/spire-server \
localauthority x509 show -output json | jq -r .prepared.authority_id) || fail-now "Failed to fetch prepared authority ID"

# Activate the authority
activated_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
activated_authority=$(docker compose exec -t root-server \
/opt/spire/bin/spire-server \
localauthority x509 activate -authorityID "${prepared_authority}" \
-output json | jq -r .activated_authority.authority_id) || fail-now "Failed to activate authority"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ check-logs() {
}

# Fetch old authority ID
old_authority=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
old_authority=$(docker compose exec -T root-server \
/opt/spire/bin/spire-server \
localauthority x509 show -output json | jq .old.authority_id -r) || fail-now "Failed to fetch old authority ID"

# Taint the old authority
docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
docker compose exec -T root-server \
/opt/spire/bin/spire-server \
localauthority x509 taint -authorityID "${old_authority}" || fail-now "Failed to taint old authority"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ get-x509-authorities-count() {
docker compose exec -T $server /opt/spire/bin/spire-server bundle show -output json | jq '.x509_authorities | length'
}

old_authority=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
old_authority=$(docker compose exec -T root-server \
/opt/spire/bin/spire-server localauthority x509 show -output json | jq .old.authority_id -r) || fail-now "Failed to get old authority"

log-debug "Old authority: $old_authority"
Expand All @@ -27,7 +27,7 @@ if [[ -z "$tainted_found" ]]; then
fail-now "Tainted authority expected"
fi

docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
docker compose exec -T root-server \
/opt/spire/bin/spire-server localauthority x509 revoke -authorityID $old_authority -output json || fail-now "Failed to revoke authority"

check-log-line root-server "X\.509 authority revoked successfully|local_authority_id=$old_authority"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ MAX_RETRIES=10
RETRY_DELAY=2 # seconds between retries

fetch-active-authority() {
docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation root-server \
docker compose exec -T root-server \
/opt/spire/bin/spire-server localauthority x509 show -output json | jq -r .active.authority_id
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ server {
# Set big numbers, to never go into regular rotations
ca_ttl = "216h"
default_x509_svid_ttl = "36h"
experimental {
feature_flags = ["forced_rotation"]
}
}

plugins {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if [[ $amount_bundles -ne 1 ]]; then
fi

# Prepare authority
prepared_authority_id=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
prepared_authority_id=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server localauthority x509 prepare -output json | jq -r .prepared_authority.authority_id)

# Verify that the prepared authority is logged
Expand All @@ -32,7 +32,7 @@ fi
new_dummy_ca_skid=$(openssl x509 -in conf/server/new_upstream_ca.crt -text | grep \
-A 1 'Subject Key Identifier' | tail -n 1 | tr -d ' ' | tr -d ':' | tr '[:upper:]' '[:lower:]')

upstream_authority_id=$(docker compose exec -T -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
upstream_authority_id=$(docker compose exec -T spire-server \
/opt/spire/bin/spire-server \
localauthority x509 show -output json | jq .prepared.upstream_authority_subject_key_id -r)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#!/bin/bash

# Fetch the prepared authority ID
prepared_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
prepared_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority x509 show -output json | jq -r .prepared.authority_id) || fail-now "Failed to fetch prepared authority ID"
upstream_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
upstream_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority x509 show -output json | jq -r .prepared.upstream_authority_subject_key_id) || fail-now "Failed to fetch prepared authority ID"

# Activate the authority
activated_authority=$(docker compose exec -t -e SPIRE_SERVER_FFLAGS=forced_rotation spire-server \
activated_authority=$(docker compose exec -t spire-server \
/opt/spire/bin/spire-server \
localauthority x509 activate -authorityID "${prepared_authority}" \
-output json | jq -r .activated_authority.authority_id) || fail-now "Failed to activate authority"
Expand Down
Loading

0 comments on commit 7abee0a

Please sign in to comment.