Merge pull request #3839 from matusf/fix/pdns-tls-on

Make TLS enabled by default for PowerDNS provider
kubernetes-sigs · Sep 5, 2023 · 0483ffd · 0483ffd
2 parents add5e92 + 721c4e2
commit 0483ffd
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 121 deletions.
diff --git a/main.go b/main.go
@@ -338,7 +338,7 @@ func main() {
 				Server:       cfg.PDNSServer,
 				APIKey:       cfg.PDNSAPIKey,
 				TLSConfig: pdns.TLSConfig{
-					TLSEnabled:            cfg.PDNSTLSEnabled,
+					SkipTLSVerify:         cfg.PDNSSkipTLSVerify,
 					CAFilePath:            cfg.TLSCA,
 					ClientCertFilePath:    cfg.TLSClientCert,
 					ClientCertKeyFilePath: cfg.TLSClientCertKey,

diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go
@@ -140,7 +140,7 @@ type Config struct {
 	OVHApiRateLimit                    int
 	PDNSServer                         string
 	PDNSAPIKey                         string `secure:"yes"`
-	PDNSTLSEnabled                     bool
+	PDNSSkipTLSVerify                  bool
 	TLSCA                              string
 	TLSClientCert                      string
 	TLSClientCertKey                   string
@@ -291,7 +291,7 @@ var defaultConfig = &Config{
 	OVHApiRateLimit:             20,
 	PDNSServer:                  "http://localhost:8081",
 	PDNSAPIKey:                  "",
-	PDNSTLSEnabled:              false,
+	PDNSSkipTLSVerify:           false,
 	TLSCA:                       "",
 	TLSClientCert:               "",
 	TLSClientCertKey:            "",
@@ -519,7 +519,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	app.Flag("ovh-api-rate-limit", "When using the OVH provider, specify the API request rate limit, X operations by seconds (default: 20)").Default(strconv.Itoa(defaultConfig.OVHApiRateLimit)).IntVar(&cfg.OVHApiRateLimit)
 	app.Flag("pdns-server", "When using the PowerDNS/PDNS provider, specify the URL to the pdns server (required when --provider=pdns)").Default(defaultConfig.PDNSServer).StringVar(&cfg.PDNSServer)
 	app.Flag("pdns-api-key", "When using the PowerDNS/PDNS provider, specify the API key to use to authorize requests (required when --provider=pdns)").Default(defaultConfig.PDNSAPIKey).StringVar(&cfg.PDNSAPIKey)
-	app.Flag("pdns-tls-enabled", "When using the PowerDNS/PDNS provider, specify whether to use TLS (default: false, requires --tls-ca, optionally specify --tls-client-cert and --tls-client-cert-key)").Default(strconv.FormatBool(defaultConfig.PDNSTLSEnabled)).BoolVar(&cfg.PDNSTLSEnabled)
+	app.Flag("pdns-skip-tls-verify", "When using the PowerDNS/PDNS provider, disable verification of any TLS certificates (optional when --provider=pdns) (default: false)").Default(strconv.FormatBool(defaultConfig.PDNSSkipTLSVerify)).BoolVar(&cfg.PDNSSkipTLSVerify)
 	app.Flag("ns1-endpoint", "When using the NS1 provider, specify the URL of the API endpoint to target (default: https://api.nsone.net/v1/)").Default(defaultConfig.NS1Endpoint).StringVar(&cfg.NS1Endpoint)
 	app.Flag("ns1-ignoressl", "When using the NS1 provider, specify whether to verify the SSL certificate (default: false)").Default(strconv.FormatBool(defaultConfig.NS1IgnoreSSL)).BoolVar(&cfg.NS1IgnoreSSL)
 	app.Flag("ns1-min-ttl", "Minimal TTL (in seconds) for records. This value will be used if the provided TTL for a service/ingress is lower than this.").IntVar(&cfg.NS1MinTTLSeconds)

diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go
@@ -205,7 +205,7 @@ var (
 		OVHApiRateLimit:             42,
 		PDNSServer:                  "http://ns.example.com:8081",
 		PDNSAPIKey:                  "some-secret-key",
-		PDNSTLSEnabled:              true,
+		PDNSSkipTLSVerify:           true,
 		TLSCA:                       "/path/to/ca.crt",
 		TLSClientCert:               "/path/to/cert.pem",
 		TLSClientCertKey:            "/path/to/key.pem",
@@ -316,7 +316,7 @@ func TestParseFlags(t *testing.T) {
 				"--ovh-api-rate-limit=42",
 				"--pdns-server=http://ns.example.com:8081",
 				"--pdns-api-key=some-secret-key",
-				"--pdns-tls-enabled",
+				"--pdns-skip-tls-verify",
 				"--oci-config-file=oci.yaml",
 				"--tls-ca=/path/to/ca.crt",
 				"--tls-client-cert=/path/to/cert.pem",
@@ -449,7 +449,7 @@ func TestParseFlags(t *testing.T) {
 				"EXTERNAL_DNS_EXCLUDE_TARGET_NET":              "1.0.0.0/9\n1.1.0.0/9",
 				"EXTERNAL_DNS_PDNS_SERVER":                     "http://ns.example.com:8081",
 				"EXTERNAL_DNS_PDNS_API_KEY":                    "some-secret-key",
-				"EXTERNAL_DNS_PDNS_TLS_ENABLED":                "1",
+				"EXTERNAL_DNS_PDNS_SKIP_TLS_VERIFY":            "1",
 				"EXTERNAL_DNS_RDNS_ROOT_DOMAIN":                "lb.rancher.cloud",
 				"EXTERNAL_DNS_TLS_CA":                          "/path/to/ca.crt",
 				"EXTERNAL_DNS_TLS_CLIENT_CERT":                 "/path/to/cert.pem",

diff --git a/provider/pdns/pdns.go b/provider/pdns/pdns.go
@@ -72,24 +72,22 @@ type PDNSConfig struct {
 
 // TLSConfig is comprised of the TLS-related fields necessary to create a new PDNSProvider
 type TLSConfig struct {
-	TLSEnabled            bool
+	SkipTLSVerify         bool
 	CAFilePath            string
 	ClientCertFilePath    string
 	ClientCertKeyFilePath string
 }
 
 func (tlsConfig *TLSConfig) setHTTPClient(pdnsClientConfig *pgo.Configuration) error {
-	if !tlsConfig.TLSEnabled {
-		log.Debug("Skipping TLS for PDNS Provider.")
-		return nil
-	}
-
 	log.Debug("Configuring TLS for PDNS Provider.")
-	if tlsConfig.CAFilePath == "" {
-		return errors.New("certificate authority file path must be specified if TLS is enabled")
-	}
-
-	tlsClientConfig, err := tlsutils.NewTLSConfig(tlsConfig.ClientCertFilePath, tlsConfig.ClientCertKeyFilePath, tlsConfig.CAFilePath, "", false, tls.VersionTLS12)
+	tlsClientConfig, err := tlsutils.NewTLSConfig(
+		tlsConfig.ClientCertFilePath,
+		tlsConfig.ClientCertKeyFilePath,
+		tlsConfig.CAFilePath,
+		"",
+		tlsConfig.SkipTLSVerify,
+		tls.VersionTLS12,
+	)
 	if err != nil {
 		return err
 	}

diff --git a/provider/pdns/pdns_test.go b/provider/pdns/pdns_test.go
@@ -721,109 +721,43 @@ func (suite *NewPDNSProviderTestSuite) TestPDNSProviderCreate() {
 }
 
 func (suite *NewPDNSProviderTestSuite) TestPDNSProviderCreateTLS() {
-	_, err := NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-		})
-	assert.Nil(suite.T(), err, "Omitted TLS Config case should raise no error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled: false,
-			},
-		})
-	assert.Nil(suite.T(), err, "Disabled TLS Config should raise no error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled:            false,
-				CAFilePath:            "/path/to/ca.crt",
-				ClientCertFilePath:    "/path/to/cert.pem",
-				ClientCertKeyFilePath: "/path/to/cert-key.pem",
-			},
-		})
-	assert.Nil(suite.T(), err, "Disabled TLS Config with additional flags should raise no error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled: true,
-			},
-		})
-	assert.Error(suite.T(), err, "Enabled TLS Config without --tls-ca should raise an error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled: true,
-				CAFilePath: "../../internal/testresources/ca.pem",
-			},
-		})
-	assert.Nil(suite.T(), err, "Enabled TLS Config with --tls-ca should raise no error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled:         true,
-				CAFilePath:         "../../internal/testresources/ca.pem",
-				ClientCertFilePath: "../../internal/testresources/client-cert.pem",
-			},
-		})
-	assert.Error(suite.T(), err, "Enabled TLS Config with --tls-client-cert only should raise an error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled:            true,
-				CAFilePath:            "../../internal/testresources/ca.pem",
-				ClientCertKeyFilePath: "../../internal/testresources/client-cert-key.pem",
-			},
-		})
-	assert.Error(suite.T(), err, "Enabled TLS Config with --tls-client-cert-key only should raise an error")
-
-	_, err = NewPDNSProvider(
-		context.Background(),
-		PDNSConfig{
-			Server:       "http://localhost:8081",
-			APIKey:       "foo",
-			DomainFilter: endpoint.NewDomainFilter([]string{""}),
-			TLSConfig: TLSConfig{
-				TLSEnabled:            true,
-				CAFilePath:            "../../internal/testresources/ca.pem",
-				ClientCertFilePath:    "../../internal/testresources/client-cert.pem",
-				ClientCertKeyFilePath: "../../internal/testresources/client-cert-key.pem",
-			},
-		})
-	assert.Nil(suite.T(), err, "Enabled TLS Config with all flags should raise no error")
+	newProvider := func(TLSConfig TLSConfig) error {
+		_, err := NewPDNSProvider(
+			context.Background(),
+			PDNSConfig{APIKey: "foo", TLSConfig: TLSConfig})
+		return err
+	}
+
+	assert.Nil(suite.T(), newProvider(TLSConfig{SkipTLSVerify: true}), "Disabled TLS Config should raise no error")
+
+	assert.Nil(suite.T(), newProvider(TLSConfig{
+		SkipTLSVerify:         true,
+		CAFilePath:            "../../internal/testresources/ca.pem",
+		ClientCertFilePath:    "../../internal/testresources/client-cert.pem",
+		ClientCertKeyFilePath: "../../internal/testresources/client-cert-key.pem",
+	}), "Disabled TLS Config with additional flags should raise no error")
+
+	assert.Nil(suite.T(), newProvider(TLSConfig{}), "Enabled TLS Config without --tls-ca should raise no error")
+
+	assert.Nil(suite.T(), newProvider(TLSConfig{
+		CAFilePath: "../../internal/testresources/ca.pem",
+	}), "Enabled TLS Config with --tls-ca should raise no error")
+
+	assert.Error(suite.T(), newProvider(TLSConfig{
+		CAFilePath:         "../../internal/testresources/ca.pem",
+		ClientCertFilePath: "../../internal/testresources/client-cert.pem",
+	}), "Enabled TLS Config with --tls-client-cert only should raise an error")
+
+	assert.Error(suite.T(), newProvider(TLSConfig{
+		CAFilePath:            "../../internal/testresources/ca.pem",
+		ClientCertKeyFilePath: "../../internal/testresources/client-cert-key.pem",
+	}), "Enabled TLS Config with --tls-client-cert-key only should raise an error")
+
+	assert.Nil(suite.T(), newProvider(TLSConfig{
+		CAFilePath:            "../../internal/testresources/ca.pem",
+		ClientCertFilePath:    "../../internal/testresources/client-cert.pem",
+		ClientCertKeyFilePath: "../../internal/testresources/client-cert-key.pem",
+	}), "Enabled TLS Config with all flags should raise no error")
 }
 
 func (suite *NewPDNSProviderTestSuite) TestPDNSRRSetToEndpoints() {