Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy pod_environment_secret to the cluster namespaces #1789

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion manifests/postgresql-operator-default-configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ configuration:
pdb_name_format: "postgres-{cluster}-pdb"
pod_antiaffinity_topology_key: "kubernetes.io/hostname"
# pod_environment_configmap: "default/my-custom-config"
# pod_environment_secret: "my-custom-secret"
# pod_environment_secret: "default/my-custom-secret"
pod_management_policy: "ordered_ready"
# pod_priority_class_name: "postgres-pod-priority"
pod_role_label: spilo-role
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/acid.zalan.do/v1/operator_configuration_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type KubernetesMetaConfiguration struct {
// TODO: use a proper toleration structure?
PodToleration map[string]string `json:"toleration,omitempty"`
PodEnvironmentConfigMap spec.NamespacedName `json:"pod_environment_configmap,omitempty"`
PodEnvironmentSecret string `json:"pod_environment_secret,omitempty"`
PodEnvironmentSecret spec.NamespacedName `json:"pod_environment_secret,omitempty"`
PodPriorityClassName string `json:"pod_priority_class_name,omitempty"`
MasterPodMoveTimeout Duration `json:"master_pod_move_timeout,omitempty"`
EnablePodAntiAffinity bool `json:"enable_pod_antiaffinity,omitempty"`
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/cluster/k8sres.go
Original file line number Diff line number Diff line change
Expand Up @@ -893,13 +893,13 @@ func (c *Cluster) getPodEnvironmentConfigMapVariables() ([]v1.EnvVar, error) {
func (c *Cluster) getPodEnvironmentSecretVariables() ([]v1.EnvVar, error) {
secretPodEnvVarsList := make([]v1.EnvVar, 0)

if c.OpConfig.PodEnvironmentSecret == "" {
if c.OpConfig.PodEnvironmentSecret.Name == "" {
return secretPodEnvVarsList, nil
}

secret, err := c.KubeClient.Secrets(c.Namespace).Get(
context.TODO(),
c.OpConfig.PodEnvironmentSecret,
c.OpConfig.PodEnvironmentSecret.Name,
metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("could not read Secret PodEnvironmentSecretName: %v", err)
Expand All @@ -910,7 +910,7 @@ func (c *Cluster) getPodEnvironmentSecretVariables() ([]v1.EnvVar, error) {
v1.EnvVar{Name: k, ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: c.OpConfig.PodEnvironmentSecret,
Name: c.OpConfig.PodEnvironmentSecret.Name,
},
Key: k,
},
Expand Down
8 changes: 6 additions & 2 deletions pkg/cluster/k8sres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,9 @@ func TestPodEnvironmentSecretVariables(t *testing.T) {
subTest: "Secret referenced by PodEnvironmentSecret does not exist",
opConfig: config.Config{
Resources: config.Resources{
PodEnvironmentSecret: "idonotexist",
PodEnvironmentSecret: spec.NamespacedName{
Name: "idonotexist",
},
},
},
err: fmt.Errorf("could not read Secret PodEnvironmentSecretName: Secret PodEnvironmentSecret not found"),
Expand All @@ -798,7 +800,9 @@ func TestPodEnvironmentSecretVariables(t *testing.T) {
subTest: "Pod environment vars reference all keys from secret configured by PodEnvironmentSecret",
opConfig: config.Config{
Resources: config.Resources{
PodEnvironmentSecret: testPodEnvironmentSecretName,
PodEnvironmentSecret: spec.NamespacedName{
Name: testPodEnvironmentSecretName,
},
},
},
envVars: []v1.EnvVar{
Expand Down
18 changes: 18 additions & 0 deletions pkg/cluster/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,24 @@ func (c *Cluster) deleteSecret(uid types.UID, secret v1.Secret) error {
return nil
}

func (c *Cluster) getSecretWithRetry(name, namespace string) (*v1.Secret, error) {
secret := &v1.Secret{}
err := retryutil.Retry(c.OpConfig.ResourceCheckInterval, c.OpConfig.ResourceCheckTimeout,
func() (bool, error) {
var err error
secret, err = c.KubeClient.Secrets(namespace).Get(
context.TODO(),
name,
metav1.GetOptions{})
if err != nil {
return false, nil
}
return true, nil
},
)
return secret, err
}

func (c *Cluster) createRoles() (err error) {
// TODO: figure out what to do with duplicate names (humans and robots) among pgUsers
return c.syncRoles()
Expand Down
62 changes: 61 additions & 1 deletion pkg/cluster/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
v1 "k8s.io/api/core/v1"
policybeta1 "k8s.io/api/policy/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)

var requireMasterRestartWhenDecreased = []string{
Expand Down Expand Up @@ -621,14 +622,18 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, patroniC
}

func (c *Cluster) syncSecrets() error {

c.logger.Info("syncing secrets")
c.setProcessName("syncing secrets")
generatedSecrets := c.generateUserSecrets()
rotationUsers := make(spec.PgUserMap)
retentionUsers := make([]string, 0)
currentTime := time.Now()

c.logger.Debug("coping PodEnvironmentSecretName if needed")
if err := c.copyPodEnvironmentSecret(); err != nil {
return err
}

for secretUsername, generatedSecret := range generatedSecrets {
secret, err := c.KubeClient.Secrets(generatedSecret.Namespace).Create(context.TODO(), generatedSecret, metav1.CreateOptions{})
if err == nil {
Expand Down Expand Up @@ -795,6 +800,61 @@ func (c *Cluster) updateSecret(
return nil
}

func (c *Cluster) copyPodEnvironmentSecret() error {
if c.OpConfig.PodEnvironmentSecret.Name == "" {
return nil
}
// Searching for a Secret within a namespace defined by the configuration
originalSecret, err := c.getSecretWithRetry(c.OpConfig.PodEnvironmentSecret.Name, c.OpConfig.PodEnvironmentSecret.Namespace)
if err != nil {
return fmt.Errorf("could not read Secret PodEnvironmentSecretName: %w", err)
}

if c.OpConfig.PodEnvironmentSecret.Namespace == c.Namespace {
return nil
}
// Attempting to find a Secret in the cluster's namespace if that namespace is not equal to the namespace defined by the configuration
secret, err := c.KubeClient.Secrets(c.Namespace).Get(
context.TODO(),
c.OpConfig.PodEnvironmentSecret.Name,
metav1.GetOptions{})
if err != nil {
if !k8sutil.ResourceNotFound(err) {
return fmt.Errorf("could not read Secret PodEnvironmentSecretName in cluster namespace: %w", err)
}
// Secret within cluster namespace not found. Let's create it
createSecret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: originalSecret.Name,
Namespace: c.Namespace,
Labels: originalSecret.Labels,
Annotations: originalSecret.Annotations,
},
Data: originalSecret.Data,
}
_, err = c.KubeClient.Secrets(c.Namespace).Create(context.TODO(), createSecret, metav1.CreateOptions{})
return k8sutil.ResourceIgnoreAlreadyExists(err)
}
// The secret exists. We need to check if it needs to be updated or not
if !reflect.DeepEqual(originalSecret.Data, secret.Data) {
patchData, err := secretDataPath(originalSecret.Data)
if err != nil {
return fmt.Errorf("could not form patch for the Secret %q: %w", c.OpConfig.PodEnvironmentSecret.Name, err)
}
_, err = c.KubeClient.Secrets(c.Namespace).Patch(
context.TODO(),
c.OpConfig.PodEnvironmentSecret.Name,
types.MergePatchType,
patchData,
metav1.PatchOptions{},
"")
if err != nil {
return fmt.Errorf("could not patch Secret %q: %w", c.OpConfig.PodEnvironmentSecret.Name, err)
}
}
return nil
}

func (c *Cluster) syncRoles() (err error) {
c.setProcessName("syncing roles")

Expand Down
71 changes: 64 additions & 7 deletions pkg/cluster/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,12 @@ package cluster

import (
"bytes"
"context"
"io/ioutil"
"net/http"
"testing"
"time"

"context"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/golang/mock/gomock"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
Expand All @@ -23,6 +18,9 @@ import (
"github.com/zalando/postgres-operator/pkg/util/config"
"github.com/zalando/postgres-operator/pkg/util/k8sutil"
"github.com/zalando/postgres-operator/pkg/util/patroni"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/fake"
)

Expand All @@ -48,7 +46,8 @@ func newFakeK8sSyncClient() (k8sutil.KubernetesClient, *fake.Clientset) {

func newFakeK8sSyncSecretsClient() (k8sutil.KubernetesClient, *fake.Clientset) {
return k8sutil.KubernetesClient{
SecretsGetter: clientSet.CoreV1(),
SecretsGetter: clientSet.CoreV1(),
NamespacesGetter: clientSet.CoreV1(),
}, clientSet
}

Expand Down Expand Up @@ -365,3 +364,61 @@ func TestUpdateSecret(t *testing.T) {
}
}
}

func TestCopyPodEnvironmentSecret(t *testing.T) {
client, _ := newFakeK8sSyncSecretsClient()

const (
clusterNamespace = metav1.NamespaceDefault
clusterName = "acid-test-cluster"
secretEnvVar = "POD_ENV_VARIABLE"
secretNamespace = "secret-ns"
secretName = "pod-environment-secret"
)

podEnvironmentSecret := &v1.Secret{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: secretNamespace,
},
StringData: map[string]string{
secretEnvVar: "data",
},
}

var cluster = New(
Config{
OpConfig: config.Config{
Resources: config.Resources{
PodEnvironmentSecret: spec.NamespacedName{
Name: secretName,
Namespace: secretNamespace,
},
ResourceCheckInterval: time.Duration(3),
ResourceCheckTimeout: time.Duration(10),
},
},
}, client, acidv1.Postgresql{}, logger, eventRecorder)
cluster.Name = clusterName
cluster.Namespace = clusterNamespace

// Run copyPodEnvironmentSecret without podEnvironmentSecret presence in the namespace
err := cluster.copyPodEnvironmentSecret()
assert.Error(t, err)

// Create a Secret in the separate namespace and try to run copyPodEnvironmentSecret again
_, err = client.Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterNamespace}}, metav1.CreateOptions{})
assert.NoError(t, err)
_, err = client.Secrets(secretNamespace).Create(context.TODO(), podEnvironmentSecret, metav1.CreateOptions{})
assert.NoError(t, err)

assert.NoError(t, cluster.copyPodEnvironmentSecret())
// Validate, that original Secret was copied to the cluster namespace
copySecret, err := client.Secrets(clusterNamespace).Get(context.TODO(), secretName, metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, podEnvironmentSecret.Data[secretEnvVar], copySecret.Data[secretEnvVar])

// Run copyPodEnvironmentSecret and validate that no error happens on retry (Secret already copied)
assert.NoError(t, cluster.copyPodEnvironmentSecret())
}
12 changes: 9 additions & 3 deletions pkg/cluster/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,21 @@ func specPatch(spec interface{}) ([]byte, error) {
}{spec})
}

func secretDataPath(data interface{}) ([]byte, error) {
return json.Marshal(struct {
Data interface{} `json:"data"`
}{data})
}

// metaAnnotationsPatch produces a JSON of the object metadata that has only the annotation
// field in order to use it in a MergePatch. Note that we don't patch the complete metadata, since
// it contains the current revision of the object that could be outdated at the time we patch.
func metaAnnotationsPatch(annotations map[string]string) ([]byte, error) {
var meta metav1.ObjectMeta
meta.Annotations = annotations
return json.Marshal(struct {
ObjMeta interface{} `json:"metadata"`
}{&meta})
}{&metav1.ObjectMeta{
Annotations: annotations,
}})
}

func (c *Cluster) logPDBChanges(old, new *policybeta1.PodDisruptionBudget, isUpdate bool, reason string) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Resources struct {
MinCPULimit string `name:"min_cpu_limit" default:"250m"`
MinMemoryLimit string `name:"min_memory_limit" default:"250Mi"`
PodEnvironmentConfigMap spec.NamespacedName `name:"pod_environment_configmap"`
PodEnvironmentSecret string `name:"pod_environment_secret"`
PodEnvironmentSecret spec.NamespacedName `name:"pod_environment_secret"`
NodeReadinessLabel map[string]string `name:"node_readiness_label" default:""`
NodeReadinessLabelMerge string `name:"node_readiness_label_merge" default:"OR"`
MaxInstances int32 `name:"max_instances" default:"-1"`
Expand Down
16 changes: 16 additions & 0 deletions pkg/util/k8sutil/k8sutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,22 @@ func ResourceNotFound(err error) bool {
return apierrors.IsNotFound(err)
}

// ResourceIgnoreNotFound return an error ignoring IsNotFound
func ResourceIgnoreNotFound(err error) error {
if ResourceNotFound(err) {
return nil
}
return err
}

// ResourceIgnoreAlreadyExists return an error ignoring IsAlreadyExists
func ResourceIgnoreAlreadyExists(err error) error {
if ResourceAlreadyExists(err) {
return nil
}
return err
}

// NewFromConfig create Kubernetes Interface using REST config
func NewFromConfig(cfg *rest.Config) (KubernetesClient, error) {
kubeClient := KubernetesClient{}
Expand Down