Add etcd s3 config secret implementation

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

docs/adrs/etcd-s3-secret.md

@@ -1,6 +1,7 @@
 # Support etcd Snapshot Configuration via Kubernetes Secret
 
 Date: 2024-02-06
+Revised: 2024-06-10
 
 ## Status
 
@@ -34,8 +35,8 @@ avoids embedding the credentials directly in the system configuration, chart val
 * We will add a `--etcd-s3-proxy` flag that can be used to set the proxy used by the S3 client. This will override the
   settings that golang's default HTTP client reads from the `HTTP_PROXY/HTTPS_PROXY/NO_PROXY` environment varibles.
 * We will add support for reading etcd snapshot S3 configuration from a Secret. The secret name will be specified via a new
-  `--etcd-s3-secret` flag, which accepts the name of the Secret in the `kube-system` namespace.
-* Presence of the `--etcd-s3-secret` flag does not imply `--etcd-s3`. If S3 is not enabled by use of the `--etcd-s3` flag,
+  `--etcd-s3-config-secret` flag, which accepts the name of the Secret in the `kube-system` namespace.
+* Presence of the `--etcd-s3-config-secret` flag does not imply `--etcd-s3`. If S3 is not enabled by use of the `--etcd-s3` flag,
   the Secret will not be used.
 * The Secret does not need to exist when K3s starts; it will be checked for every time a snapshot operation is performed.
 * Secret and CLI/config values will NOT be merged. The Secret will provide values to be used in absence of other
@@ -64,6 +65,7 @@ stringData:
   etcd-s3-access-key: "AWS_ACCESS_KEY_ID"
   etcd-s3-secret-key: "AWS_SECRET_ACCESS_KEY"
   etcd-s3-bucket: "bucket"
+  etcd-s3-folder: "folder"
   etcd-s3-region: "us-east-1"
   etcd-s3-insecure: "false"
   etcd-s3-timeout: "5m"
@@ -73,3 +75,9 @@ stringData:
 ## Consequences
 
 This will require additional documentation, tests, and QA work to validate use of secrets for s3 snapshot configuration.
+
+## Revisions
+
+#### 2024-06-10:
+* Changed flag to `etcd-s3-config-secret` to avoid confusion with `etcd-s3-secret-key`.
+* Added `etcd-s3-folder` to example Secret.

pkg/cli/cmds/etcd_snapshot.go

@@ -100,6 +100,16 @@ var EtcdSnapshotFlags = []cli.Flag{
 		Usage:       "(db) S3 folder",
 		Destination: &ServerConfig.EtcdS3Folder,
 	},
+	&cli.StringFlag{
+		Name:        "s3-proxy,etcd-s3-proxy",
+		Usage:       "(db) Proxy server to use when connecting to S3, overriding any proxy-releated environment variables",
+		Destination: &ServerConfig.EtcdS3Proxy,
+	},
+	&cli.StringFlag{
+		Name:        "s3-config-secret,etcd-s3-config-secret",
+		Usage:       "(db) Name of secret in the kube-system namespace used to configure S3, if etcd-s3 is enabled and no other etcd-s3 options are set",
+		Destination: &ServerConfig.EtcdS3ConfigSecret,
+	},
 	&cli.BoolFlag{
 		Name:        "s3-insecure,etcd-s3-insecure",
 		Usage:       "(db) Disables S3 over HTTPS",

pkg/cli/cmds/server.go

@@ -104,6 +104,8 @@ type Server struct {
 	EtcdS3BucketName         string
 	EtcdS3Region             string
 	EtcdS3Folder             string
+	EtcdS3Proxy              string
+	EtcdS3ConfigSecret       string
 	EtcdS3Timeout            time.Duration
 	EtcdS3Insecure           bool
 	ServiceLBNamespace       string
@@ -430,6 +432,16 @@ var ServerFlags = []cli.Flag{
 		Usage:       "(db) S3 folder",
 		Destination: &ServerConfig.EtcdS3Folder,
 	},
+	&cli.StringFlag{
+		Name:        "etcd-s3-proxy",
+		Usage:       "(db) Proxy server to use when connecting to S3, overriding any proxy-releated environment variables",
+		Destination: &ServerConfig.EtcdS3Proxy,
+	},
+	&cli.StringFlag{
+		Name:        "etcd-s3-config-secret",
+		Usage:       "(db) Name of secret in the kube-system namespace used to configure S3, if etcd-s3 is enabled and no other etcd-s3 options are set",
+		Destination: &ServerConfig.EtcdS3ConfigSecret,
+	},
 	&cli.BoolFlag{
 		Name:        "etcd-s3-insecure",
 		Usage:       "(db) Disables S3 over HTTPS",

pkg/cli/etcdsnapshot/etcd_snapshot.go

@@ -16,6 +16,7 @@ import (
 	"github.com/k3s-io/k3s/pkg/cli/cmds"
 	"github.com/k3s-io/k3s/pkg/clientaccess"
 	"github.com/k3s-io/k3s/pkg/cluster/managed"
+	"github.com/k3s-io/k3s/pkg/daemons/config"
 	"github.com/k3s-io/k3s/pkg/etcd"
 	"github.com/k3s-io/k3s/pkg/proctitle"
 	"github.com/k3s-io/k3s/pkg/server"
@@ -50,17 +51,20 @@ func commandSetup(app *cli.Context, cfg *cmds.Server) (*etcd.SnapshotRequest, *c
 	}
 
 	if cfg.EtcdS3 {
-		sr.S3 = &etcd.SnapshotRequestS3{}
-		sr.S3.AccessKey = cfg.EtcdS3AccessKey
-		sr.S3.Bucket = cfg.EtcdS3BucketName
-		sr.S3.Endpoint = cfg.EtcdS3Endpoint
-		sr.S3.EndpointCA = cfg.EtcdS3EndpointCA
-		sr.S3.Folder = cfg.EtcdS3Folder
-		sr.S3.Insecure = cfg.EtcdS3Insecure
-		sr.S3.Region = cfg.EtcdS3Region
-		sr.S3.SecretKey = cfg.EtcdS3SecretKey
-		sr.S3.SkipSSLVerify = cfg.EtcdS3SkipSSLVerify
-		sr.S3.Timeout = metav1.Duration{Duration: cfg.EtcdS3Timeout}
+		sr.S3 = &config.EtcdS3{
+			AccessKey:     cfg.EtcdS3AccessKey,
+			Bucket:        cfg.EtcdS3BucketName,
+			ConfigSecret:  cfg.EtcdS3ConfigSecret,
+			Endpoint:      cfg.EtcdS3Endpoint,
+			EndpointCA:    cfg.EtcdS3EndpointCA,
+			Folder:        cfg.EtcdS3Folder,
+			Insecure:      cfg.EtcdS3Insecure,
+			Proxy:         cfg.EtcdS3Proxy,
+			Region:        cfg.EtcdS3Region,
+			SecretKey:     cfg.EtcdS3SecretKey,
+			SkipSSLVerify: cfg.EtcdS3SkipSSLVerify,
+			Timeout:       metav1.Duration{Duration: cfg.EtcdS3Timeout},
+		}
 		// extend request timeout to allow the S3 operation to complete
 		timeout += cfg.EtcdS3Timeout
 	}

pkg/cli/server/server.go

@@ -32,6 +32,7 @@ import (
 	"github.com/rancher/wrangler/v3/pkg/signals"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilnet "k8s.io/apimachinery/pkg/util/net"
 	kubeapiserverflag "k8s.io/component-base/cli/flag"
 	"k8s.io/kubernetes/pkg/controlplane/apiserver/options"
@@ -186,17 +187,22 @@ func run(app *cli.Context, cfg *cmds.Server, leaderControllers server.CustomCont
 		serverConfig.ControlConfig.EtcdSnapshotCron = cfg.EtcdSnapshotCron
 		serverConfig.ControlConfig.EtcdSnapshotDir = cfg.EtcdSnapshotDir
 		serverConfig.ControlConfig.EtcdSnapshotRetention = cfg.EtcdSnapshotRetention
-		serverConfig.ControlConfig.EtcdS3 = cfg.EtcdS3
-		serverConfig.ControlConfig.EtcdS3Endpoint = cfg.EtcdS3Endpoint
-		serverConfig.ControlConfig.EtcdS3EndpointCA = cfg.EtcdS3EndpointCA
-		serverConfig.ControlConfig.EtcdS3SkipSSLVerify = cfg.EtcdS3SkipSSLVerify
-		serverConfig.ControlConfig.EtcdS3AccessKey = cfg.EtcdS3AccessKey
-		serverConfig.ControlConfig.EtcdS3SecretKey = cfg.EtcdS3SecretKey
-		serverConfig.ControlConfig.EtcdS3BucketName = cfg.EtcdS3BucketName
-		serverConfig.ControlConfig.EtcdS3Region = cfg.EtcdS3Region
-		serverConfig.ControlConfig.EtcdS3Folder = cfg.EtcdS3Folder
-		serverConfig.ControlConfig.EtcdS3Insecure = cfg.EtcdS3Insecure
-		serverConfig.ControlConfig.EtcdS3Timeout = cfg.EtcdS3Timeout
+		if cfg.EtcdS3 {
+			serverConfig.ControlConfig.EtcdS3 = &config.EtcdS3{
+				AccessKey:     cfg.EtcdS3AccessKey,
+				Bucket:        cfg.EtcdS3BucketName,
+				ConfigSecret:  cfg.EtcdS3ConfigSecret,
+				Endpoint:      cfg.EtcdS3Endpoint,
+				EndpointCA:    cfg.EtcdS3EndpointCA,
+				Folder:        cfg.EtcdS3Folder,
+				Insecure:      cfg.EtcdS3Insecure,
+				Proxy:         cfg.EtcdS3Proxy,
+				Region:        cfg.EtcdS3Region,
+				SecretKey:     cfg.EtcdS3SecretKey,
+				SkipSSLVerify: cfg.EtcdS3SkipSSLVerify,
+				Timeout:       metav1.Duration{Duration: cfg.EtcdS3Timeout},
+			}
+		}
 	} else {
 		logrus.Info("ETCD snapshots are disabled")
 	}

pkg/daemons/config/types.go

@@ -8,13 +8,13 @@ import (
 	"sort"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/k3s-io/k3s/pkg/generated/controllers/k3s.cattle.io"
 	"github.com/k3s-io/kine/pkg/endpoint"
 	"github.com/rancher/wharfie/pkg/registries"
 	"github.com/rancher/wrangler/v3/pkg/generated/controllers/core"
 	"github.com/rancher/wrangler/v3/pkg/leader"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/client-go/tools/record"
@@ -61,6 +61,21 @@ type Node struct {
 	DefaultRuntime           string
 }
 
+type EtcdS3 struct {
+	AccessKey     string          `json:"accessKey,omitempty"`
+	Bucket        string          `json:"bucket,omitempty"`
+	ConfigSecret  string          `json:"configSecret,omitempty"`
+	Endpoint      string          `json:"endpoint,omitempty"`
+	EndpointCA    string          `json:"endpointCA,omitempty"`
+	Folder        string          `json:"folder,omitempty"`
+	Proxy         string          `json:"proxy,omitempty"`
+	Region        string          `json:"region,omitempty"`
+	SecretKey     string          `json:"secretKey,omitempty"`
+	Insecure      bool            `json:"insecure,omitempty"`
+	SkipSSLVerify bool            `json:"skipSSLVerify,omitempty"`
+	Timeout       metav1.Duration `json:"timeout,omitempty"`
+}
+
 type Containerd struct {
 	Address       string
 	Log           string
@@ -215,27 +230,17 @@ type Control struct {
 	EncryptSkip              bool
 	MinTLSVersion            string
 	CipherSuites             []string
-	TLSMinVersion            uint16        `json:"-"`
-	TLSCipherSuites          []uint16      `json:"-"`
-	EtcdSnapshotName         string        `json:"-"`
-	EtcdDisableSnapshots     bool          `json:"-"`
-	EtcdExposeMetrics        bool          `json:"-"`
-	EtcdSnapshotDir          string        `json:"-"`
-	EtcdSnapshotCron         string        `json:"-"`
-	EtcdSnapshotRetention    int           `json:"-"`
-	EtcdSnapshotCompress     bool          `json:"-"`
-	EtcdListFormat           string        `json:"-"`
-	EtcdS3                   bool          `json:"-"`
-	EtcdS3Endpoint           string        `json:"-"`
-	EtcdS3EndpointCA         string        `json:"-"`
-	EtcdS3SkipSSLVerify      bool          `json:"-"`
-	EtcdS3AccessKey          string        `json:"-"`
-	EtcdS3SecretKey          string        `json:"-"`
-	EtcdS3BucketName         string        `json:"-"`
-	EtcdS3Region             string        `json:"-"`
-	EtcdS3Folder             string        `json:"-"`
-	EtcdS3Timeout            time.Duration `json:"-"`
-	EtcdS3Insecure           bool          `json:"-"`
+	TLSMinVersion            uint16   `json:"-"`
+	TLSCipherSuites          []uint16 `json:"-"`
+	EtcdSnapshotName         string   `json:"-"`
+	EtcdDisableSnapshots     bool     `json:"-"`
+	EtcdExposeMetrics        bool     `json:"-"`
+	EtcdSnapshotDir          string   `json:"-"`
+	EtcdSnapshotCron         string   `json:"-"`
+	EtcdSnapshotRetention    int      `json:"-"`
+	EtcdSnapshotCompress     bool     `json:"-"`
+	EtcdListFormat           string   `json:"-"`
+	EtcdS3                   *EtcdS3  `json:"-"`
 	ServerNodeName           string
 	VLevel                   int
 	VModule                  string

pkg/etcd/etcd.go

@@ -12,7 +12,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -25,6 +24,7 @@ import (
 	"github.com/k3s-io/k3s/pkg/daemons/config"
 	"github.com/k3s-io/k3s/pkg/daemons/control/deps"
 	"github.com/k3s-io/k3s/pkg/daemons/executor"
+	"github.com/k3s-io/k3s/pkg/etcd/s3"
 	"github.com/k3s-io/k3s/pkg/server/auth"
 	"github.com/k3s-io/k3s/pkg/util"
 	"github.com/k3s-io/k3s/pkg/version"
@@ -93,8 +93,6 @@ var (
 	ErrAddressNotSet    = errors.New("apiserver addresses not yet set")
 	ErrNotMember        = errNotMember()
 	ErrMemberListFailed = errMemberListFailed()
-
-	invalidKeyChars = regexp.MustCompile(`[^-._a-zA-Z0-9]`)
 )
 
 type NodeControllerGetter func() controllerv1.NodeController
@@ -110,8 +108,8 @@ type ETCD struct {
 	name        string
 	address     string
 	cron        *cron.Cron
-	s3          *S3
 	cancel      context.CancelFunc
+	s3          *s3.Controller
 	snapshotSem *semaphore.Weighted
 }
 
@@ -165,8 +163,6 @@ func (e *MemberListError) Is(target error) bool {
 
 func errMemberListFailed() error { return &MemberListError{} }
 
-// NewETCD creates a new value of type
-// ETCD with an initialized cron value.
 func NewETCD() *ETCD {
 	return &ETCD{
 		cron: cron.New(cron.WithLogger(cronLogger)),
@@ -390,9 +386,9 @@ func (e *ETCD) Reset(ctx context.Context, rebootstrap func() error) error {
 
 	// If asked to restore from a snapshot, do so
 	if e.config.ClusterResetRestorePath != "" {
-		if e.config.EtcdS3 {
+		if e.config.EtcdS3 != nil {
 			logrus.Infof("Retrieving etcd snapshot %s from S3", e.config.ClusterResetRestorePath)
-			if err := e.initS3IfNil(ctx); err != nil {
+			if err := e.initS3(ctx); err != nil {
 				return err
 			}
 			if err := e.s3.Download(ctx); err != nil {

pkg/etcd/etcd_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/k3s-io/k3s/pkg/daemons/config"
 	testutil "github.com/k3s-io/k3s/tests"
 	"github.com/robfig/cron/v3"
+	"github.com/sirupsen/logrus"
 	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	utilnet "k8s.io/apimachinery/pkg/util/net"
@@ -47,10 +48,12 @@ func generateTestConfig() *config.Control {
 		EtcdSnapshotName:      "etcd-snapshot",
 		EtcdSnapshotCron:      "0 */12 * * *",
 		EtcdSnapshotRetention: 5,
-		EtcdS3Endpoint:        "s3.amazonaws.com",
-		EtcdS3Region:          "us-east-1",
-		SANs:                  []string{"127.0.0.1", mustGetAddress()},
-		CriticalControlArgs:   criticalControlArgs,
+		EtcdS3: &config.EtcdS3{
+			Endpoint: "s3.amazonaws.com",
+			Region:   "us-east-1",
+		},
+		SANs:                []string{"127.0.0.1", mustGetAddress()},
+		CriticalControlArgs: criticalControlArgs,
 	}
 }
 
@@ -112,6 +115,10 @@ func Test_UnitETCD_IsInitialized(t *testing.T) {
 			want:    false,
 		},
 	}
+
+	// enable logging
+	logrus.SetLevel(logrus.DebugLevel)
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			e := NewETCD()