Skip to content

Commit

Permalink
Take ownership of an existing destination secret
Browse files Browse the repository at this point in the history
Adds a new configuration option spec.destination.clobber that when set
to true VSO will replace an existing destination secret that it does not
currently own. VSO will then take ownership of the destination
secret's life-cycle.
  • Loading branch information
benashz committed Jan 10, 2024
1 parent a38583c commit 8dc7c1c
Show file tree
Hide file tree
Showing 12 changed files with 180 additions and 35 deletions.
7 changes: 6 additions & 1 deletion api/v1beta1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ type Destination struct {
Name string `json:"name"`
// Create the destination Secret.
// If the Secret already exists this should be set to false.
Create bool `json:"create,omitempty"`
// +kubebuilder:default=false
Create bool `json:"create"`
// Clobber the destination Secret if it exists and Create is true. This is useful
// when migrating to VSO from a previous secret deployment strategy.
// +kubebuilder:default=false
Clobber bool `json:"clobber"`
// Labels to apply to the Secret. Requires Create to be set to true.
Labels map[string]string `json:"labels,omitempty"`
// Annotations to apply to the Secret. Requires Create to be set to true.
Expand Down
9 changes: 9 additions & 0 deletions chart/crds/secrets.hashicorp.com_hcpvaultsecretsapps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -70,6 +77,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
hcpAuthRef:
Expand Down
9 changes: 9 additions & 0 deletions chart/crds/secrets.hashicorp.com_vaultdynamicsecrets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -73,6 +80,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
mount:
Expand Down
9 changes: 9 additions & 0 deletions chart/crds/secrets.hashicorp.com_vaultpkisecrets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -82,6 +89,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
excludeCNFromSans:
Expand Down
9 changes: 9 additions & 0 deletions chart/crds/secrets.hashicorp.com_vaultstaticsecrets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -65,6 +72,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
hmacSecretData:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -70,6 +77,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
hcpAuthRef:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -73,6 +80,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
mount:
Expand Down
9 changes: 9 additions & 0 deletions config/crd/bases/secrets.hashicorp.com_vaultpkisecrets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -82,6 +89,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
excludeCNFromSans:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ spec:
description: Annotations to apply to the Secret. Requires Create
to be set to true.
type: object
clobber:
default: false
description: Clobber the destination Secret if it exists and Create
is true. This is useful when migrating to VSO from a previous
secret deployment strategy.
type: boolean
create:
default: false
description: Create the destination Secret. If the Secret already
exists this should be set to false.
type: boolean
Expand All @@ -65,6 +72,8 @@ spec:
set to true. Defaults to Opaque.
type: string
required:
- clobber
- create
- name
type: object
hmacSecretData:
Expand Down
1 change: 1 addition & 0 deletions docs/api/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ _Appears in:_
| --- | --- |
| `name` _string_ | Name of the Secret |
| `create` _boolean_ | Create the destination Secret. If the Secret already exists this should be set to false. |
| `clobber` _boolean_ | Clobber the destination Secret if it exists and Create is true. This is useful when migrating to VSO from a previous secret deployment strategy. |
| `labels` _object (keys:string, values:string)_ | Labels to apply to the Secret. Requires Create to be set to true. |
| `annotations` _object (keys:string, values:string)_ | Annotations to apply to the Secret. Requires Create to be set to true. |
| `type` _[SecretType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#secrettype-v1-core)_ | Type of Kubernetes Secret. Requires Create to be set to true. Defaults to Opaque. |
Expand Down
88 changes: 58 additions & 30 deletions internal/helpers/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,9 @@ func SyncSecret(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Ob
return fmt.Errorf("invalid Destination, err=%w", err)
}

var dest corev1.Secret
exists := true
if err := client.Get(ctx, key, &dest); err != nil {
if apierrors.IsNotFound(err) {
exists = false
} else {
return err
}
dest, exists, err := getSecretExists(ctx, client, key)
if err != nil {
return err
}

pruneOrphans := func() {
Expand Down Expand Up @@ -186,7 +181,7 @@ func SyncSecret(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Ob
// syncable-secret's Status, but...
dest.Data = data
logger.V(consts.LogLevelDebug).Info("Updating secret")
if err := client.Update(ctx, &dest); err != nil {
if err := client.Update(ctx, dest); err != nil {
return err
}

Expand All @@ -213,21 +208,28 @@ func SyncSecret(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Ob
}
if exists {
logger.V(consts.LogLevelDebug).Info("Found pre-existing secret",
"secret", ctrlclient.ObjectKeyFromObject(&dest))
if err := checkSecretIsOwnedByObj(&dest, references); err != nil {
return err
"secret", ctrlclient.ObjectKeyFromObject(dest))

checkOwnerShip := true
if meta.Destination.Clobber {
checkOwnerShip = hasOwnerLabels(dest)
}

if checkOwnerShip {
if err := checkSecretIsOwnedByObj(dest, references); err != nil {
return err
}
}
} else {
// secret does not exist, so we are going to create it.
dest = corev1.Secret{
dest = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: meta.Destination.Name,
Namespace: obj.GetNamespace(),
},
}
logger.V(consts.LogLevelDebug).Info("Creating new secret",
"secret", ctrlclient.ObjectKeyFromObject(&dest))
"secret", ctrlclient.ObjectKeyFromObject(dest))
}

// common setup/updates
Expand Down Expand Up @@ -257,12 +259,12 @@ func SyncSecret(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Ob

if exists {
logger.V(consts.LogLevelDebug).Info("Updating secret")
if err := client.Update(ctx, &dest); err != nil {
if err := client.Update(ctx, dest); err != nil {
return err
}
} else {
logger.V(consts.LogLevelDebug).Info("Creating secret")
if err := client.Create(ctx, &dest); err != nil {
if err := client.Create(ctx, dest); err != nil {
return err
}
}
Expand Down Expand Up @@ -298,16 +300,16 @@ func pruneOrphanSecrets(ctx context.Context, client ctrlclient.Client, obj ctrlc
//
// See NewSyncableSecretMetaData for the supported types for obj.
func CheckSecretExists(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Object) (bool, error) {
_, ok, err := getSecretExists(ctx, client, obj)
_, ok, err := getSecretExistsForObj(ctx, client, obj)
return ok, err
}

// GetSyncableSecret
func GetSyncableSecret(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Object) (*corev1.Secret, bool, error) {
return getSecretExists(ctx, client, obj)
return getSecretExistsForObj(ctx, client, obj)
}

func getSecretExists(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Object) (*corev1.Secret, bool, error) {
func getSecretExistsForObj(ctx context.Context, client ctrlclient.Client, obj ctrlclient.Object) (*corev1.Secret, bool, error) {
meta, err := common.NewSyncableSecretMetaData(obj)
if err != nil {
return nil, false, err
Expand All @@ -316,18 +318,14 @@ func getSecretExists(ctx context.Context, client ctrlclient.Client, obj ctrlclie
logger := log.FromContext(ctx).WithName("syncSecret").WithValues(
"secretName", meta.Destination.Name, "create", meta.Destination.Create)
objKey := ctrlclient.ObjectKey{Namespace: obj.GetNamespace(), Name: meta.Destination.Name}
s, err := GetSecret(ctx, client, objKey)
s, exists, err := getSecretExists(ctx, client, objKey)
if err != nil {
if apierrors.IsNotFound(err) {
logger.V(consts.LogLevelDebug).Info("Secret does not exist")
return nil, false, nil
}
// let the caller log the error
return nil, false, err
}

logger.V(consts.LogLevelDebug).Info("Secret exists")
return s, true, nil
return s, exists, nil
}

func GetSecret(ctx context.Context, client ctrlclient.Client, objKey ctrlclient.ObjectKey) (*corev1.Secret, error) {
Expand All @@ -337,22 +335,52 @@ func GetSecret(ctx context.Context, client ctrlclient.Client, objKey ctrlclient.
return &s, err
}

func getSecretExists(ctx context.Context, client ctrlclient.Client, objKey ctrlclient.ObjectKey) (*corev1.Secret, bool, error) {
s, err := GetSecret(ctx, client, objKey)
var exists bool
if err != nil {
if apierrors.IsNotFound(err) {
err = nil
}
} else {
exists = true
}
return s, exists, err
}

func hasOwnerLabels(obj ctrlclient.Object) bool {
return checkOwnerLabels(obj) == nil
}

func checkOwnerLabels(obj ctrlclient.Object) error {
// check that all owner labels are present and valid, if not return an error
// this may cause issues if we ever add new "owner" labels, but for now this check should be good enough.
var errs error

labels := obj.GetLabels()
for k, v := range OwnerLabels {
if o, ok := labels[k]; o != v || !ok {
errs = errors.Join(errs, fmt.Errorf(
"invalid owner label, key=%s, present=%t", k, ok))
}
}

return errs
}

// checkSecretIsOwnedByObj validates the Secret is owned by obj by checking its Labels and OwnerReferences.
func checkSecretIsOwnedByObj(dest *corev1.Secret, references []metav1.OwnerReference) error {
var errs error
// checking for Secret ownership relies on first checking the Secret's labels,
// then verifying that its OwnerReferences match the SyncableSecret.

// check that all owner labels are present and valid, if not return an error
// this may cause issues if we ever add new "owner" labels, but for now this check should be good enough.
key := ctrlclient.ObjectKeyFromObject(dest)
errs := checkOwnerLabels(dest)
for k, v := range OwnerLabels {
if o, ok := dest.Labels[k]; o != v || !ok {
errs = errors.Join(errs, fmt.Errorf(
"invalid owner label, key=%s, present=%t", k, ok))
}
}
// check that obj is the Secret's true Owner
key := ctrlclient.ObjectKeyFromObject(dest)
if len(dest.OwnerReferences) > 0 {
if !equality.Semantic.DeepEqual(dest.OwnerReferences, references) {
// we are not the owner, perhaps another syncable-secret resource owns this secret?
Expand Down
Loading

0 comments on commit 8dc7c1c

Please sign in to comment.