Merge pull request #1182 from alanconway/secret-keys

LOG-1725 Summarize Recognized Auth Keys.

Makefile

@@ -30,7 +30,7 @@ export CLF_TEST_INCLUDES?=
 tools: $(BINGO) $(GOLANGCI_LINT) $(JUNITREPORT) $(OPERATOR_SDK) $(OPM) $(KUSTOMIZE) $(CONTROLLER_GEN)
 
 # Should pass when run before commit.
-pre-commit: regenerate generate-bundle check
+pre-commit: clean generate-bundle check
 
 # check health of the code:
 # - Update generated code
@@ -100,7 +100,8 @@ scale-olm:
 .PHONY: scale-olm
 
 clean:
-	rm -rf bin tmp _output .target
+	rm -rf bin tmp _output .target .cache
+	find -name .kube | xargs rm -rf
 	go clean -cache -testcache ./...
 
 PATCH?=Dockerfile.patch
@@ -125,9 +126,9 @@ fmt:
 MANIFESTS=manifests/$(LOGGING_VERSION)
 
 # Do all code/CRD generation at once, with timestamp file to check out-of-date.
-GEN_TIMESTAMP=.zz_generate_timestamp
+GEN_TIMESTAMP=.target/codegen
 generate: $(GEN_TIMESTAMP)
-$(GEN_TIMESTAMP): $(shell find apis -name '*.go')  $(OPERATOR_SDK) $(CONTROLLER_GEN) $(KUSTOMIZE)
+$(GEN_TIMESTAMP): $(shell find apis -name '*.go')  $(OPERATOR_SDK) $(CONTROLLER_GEN) $(KUSTOMIZE) .target
 	@$(CONTROLLER_GEN) object paths="./apis/..."
 	@$(CONTROLLER_GEN) crd:crdVersions=v1 rbac:roleName=clusterlogging-operator paths="./..." output:crd:artifacts:config=config/crd/bases
 	@bash ./hack/generate-crd.sh

apis/logging/v1/cluster_log_forwarder_types.go

@@ -147,11 +147,39 @@ type OutputSpec struct {
 	// Secret for authentication.
 	// Name of a secret in the same namespace as the cluster logging operator.
 	//
-	// For client authentication, set secret keys `tls.crt` and `tls.key` to the client certificate and private key.
+	// All sensitive authentication information is provided via a kubernetes Secret object.
+	// A Secret is a key:value map, common keys are described here.
+	// Some output types support additional specialized keys, documented with the output-specific configuration field.
+	// All secret keys are optional, enable the security features you want by setting the relevant keys.
 	//
-	// To use your own certificate authority, set secret key `ca-bundle.crt`.
+	// Transport Layer Security (TLS)
 	//
-	// Depending on the `type` there may be other secret keys that have meaning.
+	// Using a TLS URL ('https://...' or 'ssl://...') without any secret enables basic TLS:
+	// client authenticates server using system default certificate authority.
+	//
+	// Additional TLS features are enabled by including a Secret and setting the following optional fields:
+	//
+	//   `tls.crt`: (string) File name containing a client certificate.
+	//     Enables mutual authentication. Requires `tls.key`.
+	//   `tls.key`: (string) File name containing the private key to unlock the client certificate.
+	//     Requires `tls.crt`
+	//   `passphrase`: (string) Passphrase to decode an encoded TLS private key.
+	//     Requires tls.key.
+	//   `ca-bundle.crt`: (string) File name of a custom CA for server authentication.
+	//
+	// Username and Password
+	//
+	//   `username`: (string) Authentication user name. Requires `password`.
+	//   `password`: (string) Authentication password. Requires `username`.
+	//
+	// Simple Authentication Security Layer (SASL)
+	//
+	//   `sasl.enable`: (boolean) Explicitly enable or disable SASL.
+	//     If missing, SASL is automatically enabled when any of the other `sasl.` keys are set.
+	//   `sasl.mechanisms`: (array) List of allowed SASL mechanism names.
+	//     If missing or empty, the system defaults are used.
+	//   `sasl.allow-insecure`: (boolean) Allow mechanisms that send clear-text passwords.
+	//     Default false.
 	//
 	// +optional
 	Secret *OutputSecretSpec `json:"secret,omitempty"`

apis/logging/v1/output_types.go

@@ -31,6 +31,12 @@ type OutputTypeSpec struct {
 }
 
 // Cloudwatch provides configuration for the output type `cloudwatch`
+//
+// Note: the cloudwatch output recognizes the following additional keys in the Secret:
+//
+//	`aws_secret_access_key`: AWS secret access key.
+// 	`aws_access_key_id`:AWS secret access key ID.
+//
 type Cloudwatch struct {
 	// +required
 	Region string `json:"region,omitempty"`
@@ -163,6 +169,10 @@ type Kafka struct {
 	Brokers []string `json:"brokers,omitempty"`
 }
 
+// FluentdForward does not provide additional fields, but note that
+// the fluentforward output allows this additional keys in the Secret:
+//
+//   `shared_key`: (string) Key to enable fluent-forward shared-key authentication.
 type FluentdForward struct{}
 
 type Elasticsearch struct {

config/crd/bases/logging.openshift.io_clusterlogforwarders.yaml

@@ -113,7 +113,7 @@ spec:
                   description: Output defines a destination for log messages.
                   properties:
                     cloudwatch:
-                      description: Cloudwatch provides configuration for the output type `cloudwatch`
+                      description: "Cloudwatch provides configuration for the output type `cloudwatch` \n Note: the cloudwatch output recognizes the following additional keys in the Secret: \n \t`aws_secret_access_key`: AWS secret access key. \t`aws_access_key_id`:AWS secret access key ID."
                       properties:
                         groupBy:
                           description: GroupBy defines the strategy for grouping logstreams
@@ -138,6 +138,7 @@ spec:
                           type: string
                       type: object
                     fluentdForward:
+                      description: "FluentdForward does not provide additional fields, but note that the fluentforward output allows these additional keys in the Secret: \n   `shared_key`: (string) Key to enable fluent-forward shared-key authentication."
                       type: object
                     kafka:
                       description: 'Kafka provides optional extra properties for `type: kafka`'
@@ -167,7 +168,7 @@ spec:
                       description: Name used to refer to the output from a `pipeline`.
                       type: string
                     secret:
-                      description: "Secret for authentication. Name of a secret in the same namespace as the cluster logging operator. \n For client authentication, set secret keys `tls.crt` and `tls.key` to the client certificate and private key. \n To use your own certificate authority, set secret key `ca-bundle.crt`. \n Depending on the `type` there may be other secret keys that have meaning."
+                      description: "Secret for authentication. Name of a secret in the same namespace as the cluster logging operator. \n All sensitive authentication information is provided via a kubernetes Secret object. A Secret is a key:value map, common keys are described here. Some outputs support additional specialized keys, documented with the output-specific configuration field. All secret keys are optional, enable the security features you want by setting the relevant keys. \n Transport Layer Security (TLS) \n Basic TLS is enabled by TLS URL such as 'https://...' or 'ssl://...', it does not require a Secret. For basic TLS, the client authenticates the server using the system's default certificate authority. \n Additional TLS features are enabled by selectively setting the following optional fields. \n   `tls.crt`: (string) File name of a client certificate to authenticate the client.     Enables mutual authentication. Requires `tls.key`.   `tls.key`: (string) File name containing private key to unlock the client certificate.     Requires `tls.crt`   `passphrase`: (string) Passphrase to decode an encoded TLS private key.     Requires tls.key.   `ca-bundle.crt`: (string) File name of a custom CA for server authentication. \n Username and Password \n   `username`: (string) Authentication user name. Requires `password`.   `password`: (string) Authentication password. Requires `username`. \n Simple Authentication Security Layer (SASL) \n   `sasl.enable`: (boolean) Explicitly enable or disable SASL.     If missing, SASL is automatically enabled if any of the other `sasl.` keys are set.   `sasl.mechanisms`: (array) List of allowed SASL mechanism names.     If missing or empty, the system defaults are used.   `sasl.allow-insecure`: (boolean) Allow mechanisms that send clear-text passwords.     Default false."
                       properties:
                         name:
                           description: Name of a secret in the namespace configured for log forwarder secrets.

internal/constants/constants.go

@@ -1,20 +1,42 @@
 package constants
 
+const (
+	// Keys used in ClusterLogForwarder.Output Secrets keys.
+	// Documented with OutputSpec.Secret in /apis/logging/v1/cluster_log_forwarder_types.go
+	//
+	// WARNING: changing or removing values here is a breaking API change.
+
+	// TLS keys, used by any output that supports TLS.
+
+	ClientCertKey      = "tls.crt"
+	ClientPrivateKey   = "tls.key"
+	TrustedCABundleKey = "ca-bundle.crt"
+	Passphrase         = "passphrase"
+
+	// Username/Password keys, used by any output with username/password authentication.
+
+	ClientUsername = "username"
+	ClientPassword = "password"
+
+	// SASL keys, used by any output that supports SASL.
+
+	SASLEnable        = "sasl.enable"
+	SASLMechanisms    = "sasl.mechanisms"
+	SASLAllowInsecure = "sasl.allow-insecure"
+
+	// Output-specific keys
+
+	SharedKey             = "shared_key"            // fluent forward
+	DeprecatedSaslOverSSL = "sasl_over_ssl"         // Kafka
+	AWSSecretAccessKey    = "aws_secret_access_key" //nolint:gosec
+	AWSAccessKeyID        = "aws_access_key_id"
+)
 const (
 	SingletonName = "instance"
 	OpenshiftNS   = "openshift-logging"
 	// global proxy / trusted ca bundle consts
-	ProxyName                  = "cluster"
-	SharedKey                  = "shared_key"
-	Passphrase                 = "passphrase"
-	TrustedCABundleKey         = "ca-bundle.crt"
-	SaslOverSSL                = "sasl_over_ssl"
-	AWSSecretAccessKey         = "aws_secret_access_key" //nolint:gosec
-	AWSAccessKeyID             = "aws_access_key_id"
-	ClientCertKey              = "tls.crt"
-	ClientPrivateKey           = "tls.key"
-	ClientUsername             = "username"
-	ClientPassword             = "password"
+	ProxyName = "cluster"
+
 	InjectTrustedCABundleLabel = "config.openshift.io/inject-trusted-cabundle"
 	TrustedCABundleMountFile   = "tls-ca-bundle.pem"
 	TrustedCABundleMountDir    = "/etc/pki/ca-trust/extracted/pem/"

internal/generator/fluentd/output/kafka/kafka.go

@@ -160,15 +160,9 @@ func SecurityConfig(o logging.OutputSpec, secret *corev1.Secret) []Element {
 			}
 			conf = append(conf, ca)
 		}
-		if secret != nil {
-			if _, ok := secret.Data[constants.SaslOverSSL]; ok {
-				s := SaslOverSSL(true)
-				conf = append(conf, s)
-			} else {
-				s := SaslOverSSL(false)
-				conf = append(conf, s)
-			}
-		}
+		// Try the preferred and deprecated names.
+		_, ok := security.TryKeys(secret, constants.SASLEnable, constants.DeprecatedSaslOverSSL)
+		conf = append(conf, SaslOverSSL(ok))
 	}
 	return conf
 }

internal/generator/fluentd/output/kafka/output_conf_kafka_test.go

@@ -2,6 +2,8 @@ package kafka_test
 
 import (
 	"errors"
+	"strings"
+
 	"github.com/openshift/cluster-logging-operator/internal/generator"
 	"github.com/openshift/cluster-logging-operator/internal/generator/fluentd/output/kafka"
 
@@ -133,6 +135,24 @@ var _ = Describe("Generating external kafka server output store config block", f
 			Expect(err).To(BeNil())
 			Expect(results).To(EqualTrimLines(kafkaConf))
 		})
+		It("should enable Kafka if configured", func() {
+			kafkaConf = strings.Replace(kafkaConf, "sasl_over_ssl false", "sasl_over_ssl true", 1)
+			secret := &corev1.Secret{
+				Data: map[string][]byte{"sasl.enable": nil},
+			}
+			results, err := g.GenerateConf(kafka.Conf(nil, secret, outputs[0], nil)...)
+			Expect(err).To(BeNil())
+			Expect(results).To(EqualTrimLines(kafkaConf))
+		})
+		It("should recognize deprecated SASL key", func() {
+			kafkaConf = strings.Replace(kafkaConf, "sasl_over_ssl false", "sasl_over_ssl true", 1)
+			secret := &corev1.Secret{
+				Data: map[string][]byte{"sasl_over_ssl": nil},
+			}
+			results, err := g.GenerateConf(kafka.Conf(nil, secret, outputs[0], nil)...)
+			Expect(err).To(BeNil())
+			Expect(results).To(EqualTrimLines(kafkaConf))
+		})
 	})
 
 	Context("for kafka output with multiple brokers", func() {

internal/generator/fluentd/output/security/security.go

@@ -35,62 +35,41 @@ type Passphrase struct {
 var NoSecrets = map[string]*corev1.Secret{}
 
 func HasUsernamePassword(secret *corev1.Secret) bool {
-	if secret == nil {
-		return false
-	}
-
-	if _, ok := secret.Data[constants.ClientUsername]; !ok {
-		return false
-	}
-	if _, ok := secret.Data[constants.ClientPassword]; !ok {
-		return false
-	}
-	return true
+	return HasKeys(secret, constants.ClientUsername, constants.ClientPassword)
 }
 
 func HasTLSCertAndKey(secret *corev1.Secret) bool {
-	if secret == nil {
-		return false
-	}
-
-	if _, ok := secret.Data[constants.ClientCertKey]; !ok {
-		return false
-	}
-	if _, ok := secret.Data[constants.ClientPrivateKey]; !ok {
-		return false
-	}
-	return true
+	return HasKeys(secret, constants.ClientCertKey, constants.ClientPrivateKey)
 }
 
 func HasCABundle(secret *corev1.Secret) bool {
-	if secret == nil {
-		return false
-	}
-
-	if _, ok := secret.Data[constants.TrustedCABundleKey]; !ok {
-		return false
-	}
-	return true
+	return HasKeys(secret, constants.TrustedCABundleKey)
 }
 
 func HasSharedKey(secret *corev1.Secret) bool {
-	if secret == nil {
-		return false
-	}
-
-	if _, ok := secret.Data[constants.SharedKey]; !ok {
-		return false
-	}
-	return true
+	return HasKeys(secret, constants.SharedKey)
 }
 
 func HasPassphrase(secret *corev1.Secret) bool {
+	return HasKeys(secret, constants.Passphrase)
+}
+
+// GetKey if found return value and ok=true, else ok=false
+func GetKey(secret *corev1.Secret, key string) (data []byte, ok bool) {
 	if secret == nil {
-		return false
+		return nil, false
 	}
+	data, ok = secret.Data[key]
+	return data, ok
+}
 
-	if _, ok := secret.Data[constants.Passphrase]; !ok {
-		return false
+// HasKeys true if all keys are present.
+func HasKeys(secret *corev1.Secret, keys ...string) bool {
+	for _, k := range keys {
+		_, ok := GetKey(secret, k)
+		if !ok {
+			return false
+		}
 	}
 	return true
 }
@@ -99,6 +78,18 @@ func SecretPath(name string, file string) string {
 	return fmt.Sprintf("'%s'", filepath.Join("/var/run/ocp-collector/secrets", name, file))
 }
 
+// TryKeys try keys in turn return data for fist one present with ok=true.
+// If none present return ok=false.
+func TryKeys(secret *corev1.Secret, keys ...string) (data []byte, ok bool) {
+	for _, k := range keys {
+		data, ok := GetKey(secret, k)
+		if ok {
+			return data, true
+		}
+	}
+	return nil, false
+}
+
 func GetFromSecret(secret *corev1.Secret, name string) string {
 	if secret != nil {
 		return string(secret.Data[name])

internal/generator/fluentd/output/security/security_test.go

@@ -18,6 +18,54 @@ var _ = Describe("Helpers for outputLabelConf", func() {
 			Data: map[string][]byte{},
 		}
 	})
+	Context("#Has Keys", func() {
+		It("should be true if and only if all keys present", func() {
+			secret.Data = map[string][]byte{"a": nil, "b": nil, "c": nil, "d": nil}
+			Expect(HasKeys(secret, "a", "b", "c")).To(BeTrue())
+			secret.Data = map[string][]byte{"a": nil, "c": nil, "d": nil}
+			Expect(HasKeys(secret, "a", "b", "c")).To(BeFalse())
+			// nil/empty cases.
+			Expect(HasKeys(nil, "a", "b", "c")).To(BeFalse())
+			Expect(HasKeys(&corev1.Secret{}, "a", "b", "c")).To(BeFalse())
+		})
+	})
+	Context("#HasKeys", func() {
+		It("should be true if and only if all keys present", func() {
+			secret.Data = map[string][]byte{"a": nil, "b": nil, "c": nil, "d": nil}
+			Expect(HasKeys(secret, "a", "b", "c")).To(BeTrue())
+			secret.Data = map[string][]byte{"a": nil, "c": nil, "d": nil}
+			Expect(HasKeys(secret, "a", "b", "c")).To(BeFalse())
+			// nil/empty cases.
+			Expect(HasKeys(nil, "a", "b", "c")).To(BeFalse())
+			Expect(HasKeys(&corev1.Secret{}, "a", "b", "c")).To(BeFalse())
+		})
+	})
+	Context("#TryKeys", func() {
+		It("should return first key present", func() {
+
+			secret.Data = map[string][]byte{"x": {1}, "y": {2}}
+			_, ok := TryKeys(secret, "a", "b", "c")
+			Expect(ok).To(BeFalse())
+
+			v, ok := TryKeys(secret, "a", "b", "x")
+			Expect(ok).To(BeTrue())
+			Expect(v).To(Equal([]byte{1}))
+
+			v, ok = TryKeys(secret, "x", "b", "c")
+			Expect(ok).To(BeTrue())
+			Expect(v).To(Equal([]byte{1}))
+
+			v, ok = TryKeys(secret, "y", "x")
+			Expect(ok).To(BeTrue())
+			Expect(v).To(Equal([]byte{2}))
+
+			// nil/empty cases.
+			_, ok = TryKeys(nil, "a", "b", "c")
+			Expect(ok).To(BeFalse())
+			_, ok = TryKeys(nil, "a", "b", "c")
+			Expect(ok).To(BeFalse())
+		})
+	})
 	Context("#HasCABundle", func() {
 		It("should recognize when the output secret is nil", func() {
 			secret = nil