From 2e8ef7b2128e8576236ed6ea204e0e68b7f3e6ab Mon Sep 17 00:00:00 2001 From: Viktor Date: Tue, 14 May 2024 16:24:17 +0300 Subject: [PATCH] Only use immutable tls secret (#76) * shoot-rsyslog-relp admission rejects mutable tls secret * Adapt unit test * Remove redundant validation in osc webhook * Remove unused parameters * Adapt shoot_test.go to use immutable secret * Adapt e2e test to use immutable secret * Addressing plkokanov's feedback * Adapt example * Adapt documentation * Address grammar and term feedback --- docs/usage/configuration.md | 5 ++- example/secret-rsyslog-tls-certs.yaml | 1 + pkg/admission/validator/shoot_test.go | 1 + pkg/utils/utils.go | 4 +++ pkg/utils/utils_test.go | 20 +++++++---- pkg/webhook/operatingsystemconfig/ensurer.go | 2 +- pkg/webhook/operatingsystemconfig/rsyslog.go | 33 ++----------------- .../testdata/tls/rsyslog-relp-secret.yaml | 1 + 8 files changed, 28 insertions(+), 39 deletions(-) diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 968732b5..ec334969 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -123,16 +123,19 @@ loggingRules: ### Securing the Communication to the Target Server with TLS -The communication to the target server is not encrypted by default. To enable encryption, set the `.tls.enabled` field in the `shoot-rsyslog-relp` extension configuration to `true`. In this case, a secret which contains the TLS certificates used to establish the TLS connection to the server must be created in the same project namespace as your Shoot. +The communication to the target server is not encrypted by default. To enable encryption, set the `.tls.enabled` field in the `shoot-rsyslog-relp` extension configuration to `true`. In this case, an immutable secret which contains the TLS certificates used to establish the TLS connection to the server must be created in the same project namespace as your Shoot. An example Secret is given below: +> **Note:** The secret must be immutable + ```yaml kind: Secret apiVersion: v1 metadata: name: rsyslog-relp-tls-v1 namespace: garden-foo +immutable: true data: ca: | -----BEGIN BEGIN RSA PRIVATE KEY----- diff --git a/example/secret-rsyslog-tls-certs.yaml b/example/secret-rsyslog-tls-certs.yaml index e9ea4f0e..ea8b12b2 100644 --- a/example/secret-rsyslog-tls-certs.yaml +++ b/example/secret-rsyslog-tls-certs.yaml @@ -8,6 +8,7 @@ apiVersion: v1 metadata: name: rsyslog-relp-tls namespace: garden-local +immutable: true stringData: ca: | -----BEGIN CERTIFICATE----- diff --git a/pkg/admission/validator/shoot_test.go b/pkg/admission/validator/shoot_test.go index 53200e06..1cc739aa 100644 --- a/pkg/admission/validator/shoot_test.go +++ b/pkg/admission/validator/shoot_test.go @@ -222,6 +222,7 @@ tls: Name: "rsyslog-secret", Namespace: "bar", }, + Immutable: ptr.To(true), Data: map[string][]byte{ "ca": []byte("data"), "crt": []byte("data"), diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index f7db8759..7d8b9cd9 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -9,6 +9,7 @@ import ( "strings" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/gardener/gardener-extension-shoot-rsyslog-relp/pkg/constants" @@ -34,6 +35,9 @@ func ValidateRsyslogRelpSecret(secret *corev1.Secret) error { if _, ok := secret.Data[constants.RsyslogPrivateKeyKey]; !ok { return fmt.Errorf("secret %s is missing %s value", key.String(), constants.RsyslogPrivateKeyKey) } + if !ptr.Deref(secret.Immutable, false) { + return fmt.Errorf("secret %s must be immutable", key.String()) + } if len(secret.Data) != 3 { return fmt.Errorf("secret %s should have only three data entries", key.String()) } diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index b321fffe..69b33b8b 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -22,7 +22,7 @@ var _ = Describe("Utils", func() { }) DescribeTable("#ValidateRsyslogRelpSecret", - func(caData, crtData, keyData, extraData []byte, matcher types.GomegaMatcher) { + func(caData, crtData, keyData, extraData []byte, immutable bool, matcher types.GomegaMatcher) { var data = map[string][]byte{} if len(caData) > 0 { @@ -43,35 +43,41 @@ var _ = Describe("Utils", func() { Name: "rsyslog-secret", Namespace: "foo", }, - Data: data, + Immutable: &immutable, + Data: data, } Expect(ValidateRsyslogRelpSecret(secret)).To(matcher) }, Entry( "should return error if secret does not contain 'ca' data entry", - nil, nil, nil, nil, + nil, nil, nil, nil, true, MatchError(ContainSubstring("secret foo/rsyslog-secret is missing ca value")), ), Entry( "should return error if secret does not contain 'crt' data entry", - []byte("caData"), nil, nil, nil, + []byte("caData"), nil, nil, nil, true, MatchError(ContainSubstring("secret foo/rsyslog-secret is missing crt value")), ), Entry( "should return error if secret does not contain 'key' data entry", - []byte("caData"), []byte("crtData"), nil, nil, + []byte("caData"), []byte("crtData"), nil, nil, true, MatchError(ContainSubstring("secret foo/rsyslog-secret is missing key value")), ), Entry( "should not return error if secret is valid", - []byte("caData"), []byte("crtData"), []byte("keyData"), nil, + []byte("caData"), []byte("crtData"), []byte("keyData"), nil, true, Succeed(), ), Entry( "should return error if secret does not contain 'tls' data entry", - []byte("caData"), []byte("crtData"), []byte("tlsData"), []byte("extraData"), + []byte("caData"), []byte("crtData"), []byte("tlsData"), []byte("extraData"), true, MatchError(ContainSubstring("secret foo/rsyslog-secret should have only three data entries")), ), + Entry( + "should return error if secret is mutable", + []byte("caData"), []byte("crtData"), []byte("tlsData"), []byte("extraData"), false, + MatchError(ContainSubstring("secret foo/rsyslog-secret must be immutable")), + ), ) }) diff --git a/pkg/webhook/operatingsystemconfig/ensurer.go b/pkg/webhook/operatingsystemconfig/ensurer.go index 73f5f670..bb3747bf 100644 --- a/pkg/webhook/operatingsystemconfig/ensurer.go +++ b/pkg/webhook/operatingsystemconfig/ensurer.go @@ -70,7 +70,7 @@ func (e *ensurer) EnsureAdditionalFiles(ctx context.Context, gctx gcontext.Garde } } - rsyslogFiles, err := getRsyslogFiles(ctx, e.client, extension.Namespace, shootRsyslogRelpConfig, cluster) + rsyslogFiles, err := getRsyslogFiles(shootRsyslogRelpConfig, cluster) if err != nil { return err } diff --git a/pkg/webhook/operatingsystemconfig/rsyslog.go b/pkg/webhook/operatingsystemconfig/rsyslog.go index c4f2bb04..c2abd84d 100644 --- a/pkg/webhook/operatingsystemconfig/rsyslog.go +++ b/pkg/webhook/operatingsystemconfig/rsyslog.go @@ -6,7 +6,6 @@ package operatingsystemconfig import ( "bytes" - "context" _ "embed" "fmt" "strconv" @@ -15,15 +14,11 @@ import ( "github.com/Masterminds/sprig" extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" - gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" v1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" gardenerutils "github.com/gardener/gardener/pkg/utils" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/gardener/gardener-extension-shoot-rsyslog-relp/pkg/apis/rsyslog" "github.com/gardener/gardener-extension-shoot-rsyslog-relp/pkg/constants" @@ -105,14 +100,14 @@ func init() { } } -func getRsyslogFiles(ctx context.Context, c client.Client, namespace string, rsyslogRelpConfig *rsyslog.RsyslogRelpConfig, cluster *extensionscontroller.Cluster) ([]extensionsv1alpha1.File, error) { +func getRsyslogFiles(rsyslogRelpConfig *rsyslog.RsyslogRelpConfig, cluster *extensionscontroller.Cluster) ([]extensionsv1alpha1.File, error) { var rsyslogFiles []extensionsv1alpha1.File rsyslogValues := getRsyslogValues(rsyslogRelpConfig, cluster) if rsyslogRelpConfig.TLS != nil && rsyslogRelpConfig.TLS.Enabled { rsyslogValues["tls"] = getRsyslogTLSValues(rsyslogRelpConfig) - rsyslogTLSFiles, err := getRsyslogTLSFiles(ctx, c, cluster, *rsyslogRelpConfig.TLS.SecretReferenceName, namespace) + rsyslogTLSFiles, err := getRsyslogTLSFiles(cluster, *rsyslogRelpConfig.TLS.SecretReferenceName) if err != nil { return nil, err } @@ -215,20 +210,12 @@ func getRsyslogTLSValues(rsyslogRelpConfig *rsyslog.RsyslogRelpConfig) map[strin } } -func getRsyslogTLSFiles(ctx context.Context, c client.Client, cluster *extensionscontroller.Cluster, secretRefName, namespace string) ([]extensionsv1alpha1.File, error) { +func getRsyslogTLSFiles(cluster *extensionscontroller.Cluster, secretRefName string) ([]extensionsv1alpha1.File, error) { ref := v1beta1helper.GetResourceByName(cluster.Shoot.Spec.Resources, secretRefName) if ref == nil || ref.ResourceRef.Kind != "Secret" { return nil, fmt.Errorf("failed to find referenced resource with name %s and kind Secret", secretRefName) } - // TODO(plkokanov): Remove this validation once the referenced secret in the project in - // the garden cluster is forced to be immutable. In that case updating the secret will require - // creating a new secret object and editing the shoot.spec.resources to refer to that new secret. - // This will trigger the secret validation in the shoot-rsyslog-relp admission. - if err := validateReferencedSecret(ctx, c, ref, secretRefName, namespace); err != nil { - return nil, fmt.Errorf("referenced secret is not valid: %w", err) - } - refSecretName := v1beta1constants.ReferencedResourcesPrefix + ref.ResourceRef.Name return []extensionsv1alpha1.File{ { @@ -298,17 +285,3 @@ func computeLogFilters(loggingRules []rsyslog.LoggingRule) []string { return filters } - -func validateReferencedSecret(ctx context.Context, c client.Client, ref *gardencorev1beta1.NamedResourceReference, secretRefName, namespace string) error { - refSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: ref.ResourceRef.Name, - Namespace: namespace, - }, - } - if err := extensionscontroller.GetObjectByReference(ctx, c, &ref.ResourceRef, namespace, refSecret); err != nil { - return fmt.Errorf("failed to read referenced secret %s%s for reference %s", v1beta1constants.ReferencedResourcesPrefix, ref.ResourceRef.Name, secretRefName) - } - - return utils.ValidateRsyslogRelpSecret(refSecret) -} diff --git a/test/common/testdata/tls/rsyslog-relp-secret.yaml b/test/common/testdata/tls/rsyslog-relp-secret.yaml index ceefb187..3e0a2ff2 100644 --- a/test/common/testdata/tls/rsyslog-relp-secret.yaml +++ b/test/common/testdata/tls/rsyslog-relp-secret.yaml @@ -8,6 +8,7 @@ apiVersion: v1 metadata: name: rsyslog-relp-tls namespace: garden-local +immutable: true stringData: ca: | -----BEGIN CERTIFICATE-----