migrate experimental-peer-skip-client-san-verification flag to feature gate

server/embed/config.go

@@ -452,6 +452,12 @@ type Config struct {
 	// Defaults to 0.
 	ExperimentalDistributedTracingSamplingRatePerMillion int `json:"experimental-distributed-tracing-sampling-rate"`
 
+	// ExperimentalPeerSkipClientSanVerification determines whether to skip verification of SAN field
+	// in client certificate for peer connections.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: Delete in v3.7
+	ExperimentalPeerSkipClientSanVerification bool `json:"experimental-peer-skip-client-san-verification"`
+
 	// Logger is logger options: currently only supports "zap".
 	// "capnslog" is removed in v3.5.
 	Logger string `json:"logger"`
@@ -755,7 +761,8 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 	fs.Var(flags.NewStringsValue(""), "peer-cert-allowed-cn", "Comma-separated list of allowed CNs for inter-peer TLS authentication.")
 	fs.Var(flags.NewStringsValue(""), "peer-cert-allowed-hostname", "Comma-separated list of allowed SAN hostnames for inter-peer TLS authentication.")
 	fs.Var(flags.NewStringsValue(""), "cipher-suites", "Comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).")
-	fs.BoolVar(&cfg.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
+	fs.BoolVar(&cfg.ExperimentalPeerSkipClientSanVerification, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.Deprecated in v3.6 and will be decommissioned in v3.7. Use peer-skip-client-san-verification instead")
+	fs.BoolVar(&cfg.PeerTLSInfo.SkipClientSANVerify, "peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
 	fs.StringVar(&cfg.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.")
 	fs.StringVar(&cfg.TlsMaxVersion, "tls-max-version", string(tlsutil.TLSVersionDefault), "Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty defers to Go).")
 

server/etcdmain/config.go

@@ -199,6 +199,9 @@ func (cfg *config) parse(arguments []string) error {
 	if cfg.ec.FlagsExplicitlySet["experimental-bootstrap-defrag-threshold-megabytes"] {
 		cfg.ec.BootstrapDefragThresholdMegabytes = cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes
 	}
+	if cfg.ec.FlagsExplicitlySet["experimental-peer-skip-client-san-verification"] {
+		cfg.ec.PeerTLSInfo.SkipClientSANVerify = cfg.ec.ExperimentalPeerSkipClientSanVerification
+	}
 
 	if cfg.ec.FlagsExplicitlySet["experimental-max-learners"] {
 		cfg.ec.MaxLearners = cfg.ec.ExperimentalMaxLearners

server/etcdmain/config_test.go

@@ -21,6 +21,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -1152,3 +1153,100 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 		})
 	}
 }
+
+// TestPeerSkipClientSanVerificationFlagMigration tests the migration from
+// --experimental-peer-skip-client-san-verification to --peer-skip-client-san-verification
+// TODO: delete in v3.7
+func TestPeerSkipClientSanVerificationFlagMigration(t *testing.T) {
+	testCases := []struct {
+		name                                      string
+		peerSkipClientSanVerification             string
+		experimentalPeerSkipClientSanVerification string
+		useConfigFile                             bool
+		expectErr                                 bool
+		expectedPeerSkipClientSanVerification     bool
+	}{
+		{
+			name:                          "set both experimental flag and non experimental flag,experimental = false,non experimental = true",
+			peerSkipClientSanVerification: "true",
+			experimentalPeerSkipClientSanVerification: "false",
+			expectedPeerSkipClientSanVerification:     false,
+		},
+		{
+			name:                          "set both experimental flag and non experimental flag,experimental = true,non experimental = false",
+			peerSkipClientSanVerification: "false",
+			experimentalPeerSkipClientSanVerification: "true",
+			expectedPeerSkipClientSanVerification:     true,
+		},
+		{
+			name: "can set experimental flag to true",
+			experimentalPeerSkipClientSanVerification: "true",
+			expectedPeerSkipClientSanVerification:     true,
+		},
+		{
+			name: "can set experimental flag to false",
+			experimentalPeerSkipClientSanVerification: "false",
+			expectedPeerSkipClientSanVerification:     false,
+		},
+		{
+			name:                                  "can set non experimental flag to true",
+			peerSkipClientSanVerification:         "true",
+			expectedPeerSkipClientSanVerification: true,
+		},
+		{
+			name:                                  "can set non experimental flag to false",
+			peerSkipClientSanVerification:         "false",
+			expectedPeerSkipClientSanVerification: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cmdLineArgs := []string{}
+			type securityConfig struct {
+				PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification"`
+			}
+			yc := struct {
+				ExperimentalPeerSkipClientSanVerification bool `json:"experimental-peer-skip-client-san-verification,omitempty"`
+				PeerSkipClientSanVerification             bool `json:"peer-skip-client-san-verification,omitempty"`
+			}{}
+
+			if tc.peerSkipClientSanVerification != "" {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--peer-skip-client-san-verification=%s", tc.peerSkipClientSanVerification))
+				val, err := strconv.ParseBool(tc.peerSkipClientSanVerification)
+				if err != nil {
+					t.Fatal(err)
+				}
+				yc.PeerSkipClientSanVerification = val
+			}
+
+			if tc.experimentalPeerSkipClientSanVerification != "" {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-peer-skip-client-san-verification=%s", tc.experimentalPeerSkipClientSanVerification))
+				val, err := strconv.ParseBool(tc.experimentalPeerSkipClientSanVerification)
+				if err != nil {
+					t.Fatal(err)
+				}
+				yc.ExperimentalPeerSkipClientSanVerification = val
+			}
+
+			cfgFromCmdLine := newConfig()
+			errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs)
+
+			if tc.expectErr {
+				if errFromCmdLine == nil {
+					t.Fatal("expect parse error")
+				}
+				return
+			}
+
+			if errFromCmdLine != nil {
+				t.Fatal(errFromCmdLine)
+			}
+			if cfgFromCmdLine.ec.PeerTLSInfo.SkipClientSANVerify != tc.expectedPeerSkipClientSanVerification {
+				t.Errorf("expected SkipClientSANVerify=%v, got %v",
+					tc.expectedPeerSkipClientSanVerification,
+					cfgFromCmdLine.ec.PeerTLSInfo.SkipClientSANVerify)
+			}
+		})
+	}
+}

server/etcdmain/help.go

@@ -292,7 +292,9 @@ Experimental feature:
   --compaction-batch-limit 1000
     CompactionBatchLimit sets the maximum revisions deleted in each compaction batch.
   --experimental-peer-skip-client-san-verification 'false'
-    Skip verification of SAN field in client certificate for peer connections.
+    Skip verification of SAN field in client certificate for peer connections.Deprecated in v3.6 and will be decommissioned in v3.7. Use 'peer-skip-client-san-verification' instead.
+  --peer-skip-client-san-verification 'false'
+    Skip verification of SAN field in client certificate for peer connections.When both flags are set, --experimental-peer-skip-client-san-verification takes precedence over this.
   --experimental-watch-progress-notify-interval '10m'
     Duration of periodical watch progress notification. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'watch-progress-notify-interval' instead.
   --watch-progress-notify-interval '10m'