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

feat: support secret as a target #193

Merged
merged 1 commit into from
Oct 18, 2023
Merged
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
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,12 @@ ensure-cert-manager: ensure-kind $(BINDIR)/kubeconfig.yaml | $(BINDIR)/helm-$(HE
.PHONY: ensure-trust-manager
ensure-trust-manager: ensure-kind kind-load ensure-cert-manager | $(BINDIR)/helm-$(HELM_VERSION)/helm ## ensure trust-manager is available on cluster, built from local checkout
$(BINDIR)/helm-$(HELM_VERSION)/helm uninstall --kubeconfig $(BINDIR)/kubeconfig.yaml -n cert-manager trust-manager || :
$(BINDIR)/helm-$(HELM_VERSION)/helm upgrade --kubeconfig $(BINDIR)/kubeconfig.yaml -i -n cert-manager trust-manager deploy/charts/trust-manager/. --set image.tag=latest --set defaultTrustPackage.tag=latest$(DEBIAN_TRUST_PACKAGE_SUFFIX) --set app.logLevel=2 --wait
$(BINDIR)/helm-$(HELM_VERSION)/helm upgrade --kubeconfig $(BINDIR)/kubeconfig.yaml -i -n cert-manager trust-manager deploy/charts/trust-manager/. \
--set image.tag=latest \
--set defaultTrustPackage.tag=latest$(DEBIAN_TRUST_PACKAGE_SUFFIX) \
--set app.logLevel=2 \
--set secretTargets.enabled=true --set secretTargets.authorizedSecretsAll=true \
--wait

# When running in our CI environment the Docker network's subnet choice
# causees issues with routing.
Expand Down
3 changes: 3 additions & 0 deletions deploy/charts/trust-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,8 @@ Kubernetes: `>= 1.25.0-0`
| nodeSelector | object | `{"kubernetes.io/os":"linux"}` | Configure the nodeSelector; defaults to any Linux node (trust-manager doesn't support Windows nodes) |
| replicaCount | int | `1` | Number of replicas of trust-manager to run. |
| resources | object | `{}` | |
| secretTargets.authorizedSecrets | list | `[]` | A list of secret names which trust-manager will be permitted to read and write across all namespaces. These will be the only allowable Secrets that can be used as targets. If the list is empty, trust-manager will not be able to write to secrets and will only be able to read secrets in the trust namespace for use as sources. |
| secretTargets.authorizedSecretsAll | bool | `false` | If set to true, grant read/write permission to all secrets across the cluster. Use with caution! When set, ignores authorizedSecrets list setting. |
| secretTargets.enabled | bool | `false` | If set to true, enable writing trust bundles to Kubernetes Secrets as a target. trust-manager can only write to secrets which are explicitly allowed. - see either authorizedSecrets or authorizedSecretsAll. |
| tolerations | list | `[]` | List of Kubernetes Tolerations; see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#toleration-v1-core |
| topologySpreadConstraints | list | `[]` | List of Kubernetes TopologySpreadConstraints; see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#topologyspreadconstraint-v1-core |
22 changes: 22 additions & 0 deletions deploy/charts/trust-manager/templates/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,25 @@ rules:
resources:
- "events"
verbs: ["create", "patch"]

- apiGroups:
- ""
resources:
- "secrets"
verbs: ["create", "list", "watch"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SgtCoDFish shouldn't this also be part of the .Values.secretTargets.enabled section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still by default grant trust-manager access to read ALL secret resources.

Copy link
Contributor

@erikgb erikgb Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we must conditionally watch secrets from the controller if we add a condition here. And that might be the reason behind this bug.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a WIP PR to fix this: #207. My Tuesday (tomorrow) is filled with meetings, so feel free to pick it up. My gut feeling says that this new feature needs more work....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's my bad... thought I checked the permissions and clearly wasn't thorough / careful enough in my efforts to move this forwards. Sorry.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming that this was not the intended behavior @SgtCoDFish.

{{- if .Values.secretTargets.enabled }}
{{- if .Values.secretTargets.authorizedSecretsAll }}
- apiGroups:
- ""
resources:
- "secrets"
verbs: ["get", "update", "delete", "patch"]
{{- else if .Values.secretTargets.authorizedSecrets }}
- apiGroups:
- ""
resources:
- "secrets"
verbs: ["get", "update", "delete", "patch"]
resourceNames: {{ .Values.secretTargets.authorizedSecrets | toYaml | nindent 2 }}
{{- end -}}
{{- end -}}
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@ spec:
type: object
additionalProperties:
type: string
secret:
description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace.
type: object
required:
- key
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
type: string
status:
description: Status of the Bundle. This is set and managed automatically.
type: object
Expand Down Expand Up @@ -220,6 +229,15 @@ spec:
type: object
additionalProperties:
type: string
secret:
description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace.
type: object
required:
- key
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
type: string
served: true
storage: true
subresources:
Expand Down
14 changes: 14 additions & 0 deletions deploy/charts/trust-manager/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,20 @@ app:
# -- If false, disables the default seccomp profile, which might be required to run on certain platforms
seccompProfileEnabled: true

secretTargets:
# -- If set to true, enable writing trust bundles to Kubernetes Secrets as a target.
# trust-manager can only write to secrets which are explicitly allowed.
# - see either authorizedSecrets or authorizedSecretsAll.
enabled: false
# -- If set to true, grant read/write permission to all secrets across the cluster. Use with caution!
# When set, ignores authorizedSecrets list setting.
authorizedSecretsAll: false
# -- A list of secret names which trust-manager will be permitted to read and write across all namespaces.
# These will be the only allowable Secrets that can be used as targets.
# If the list is empty, trust-manager will not be able to write to secrets and will only be able to
# read secrets in the trust namespace for use as sources.
authorizedSecrets: []

resources: {}
# -- Kubernetes pod resource limits for trust.
# limits:
Expand Down
18 changes: 18 additions & 0 deletions deploy/crds/trust.cert-manager.io_bundles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,15 @@ spec:
type: object
additionalProperties:
type: string
secret:
description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace.
type: object
required:
- key
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
type: string
status:
description: Status of the Bundle. This is set and managed automatically.
type: object
Expand Down Expand Up @@ -219,6 +228,15 @@ spec:
type: object
additionalProperties:
type: string
secret:
description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace.
type: object
required:
- key
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
type: string
served: true
storage: true
subresources:
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/trust/v1alpha1/types_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ type BundleTarget struct {
// data will be synced to.
ConfigMap *KeySelector `json:"configMap,omitempty"`

// Secret is the target Secret that all Bundle source data will be synced to.
// Using Secrets as targets is only supported if enabled at trust-manager startup.
// By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace.
Secret *KeySelector `json:"secret,omitempty"`

// AdditionalFormats specifies any additional formats to write to the target
// +optional
AdditionalFormats *AdditionalFormats `json:"additionalFormats,omitempty"`
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go

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

115 changes: 94 additions & 21 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type bundle struct {
// a direct Kubernetes client (only used for CSA to CSA migration)
directClient client.Client

// targetCache is a cache.Cache that holds cached ConfigMap
// targetCache is a cache.Cache that holds cached ConfigMap and Secret
// resources that are used as targets for Bundles.
targetCache client.Reader

Expand Down Expand Up @@ -204,7 +204,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
}

// Find all old existing ConfigMap targetResources.
{
if bundle.Spec.Target.ConfigMap != nil {
configMapList := &metav1.PartialObjectMetadataList{
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
Expand Down Expand Up @@ -250,32 +250,105 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
}
}

// Find all old existing Secret targetResources.
if bundle.Spec.Target.Secret != nil {
secretLists := &metav1.PartialObjectMetadataList{
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
Kind: "Secret",
},
}
err := b.targetCache.List(ctx, secretLists, &client.ListOptions{
LabelSelector: labels.SelectorFromSet(map[string]string{
trustapi.BundleLabelKey: bundle.Name,
}),
})
if err != nil {
log.Error(err, "failed to list secrets")
b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SecretListError", "Failed to list secrets: %s", err)
return ctrl.Result{}, nil, fmt.Errorf("failed to list Secrets: %w", err)
}

for _, secret := range secretLists.Items {
key := types.NamespacedName{
Name: secret.Name,
Namespace: secret.Namespace,
}

secretLog := log.WithValues("secret", key)

if _, ok := targetResources[key]; ok {
// This Secret is still a target, so we don't need to remove it.
continue
}

// Don't reconcile target for Secrets that are being deleted.
if secret.GetDeletionTimestamp() != nil {
secretLog.V(2).WithValues("deletionTimestamp", secret.GetDeletionTimestamp()).Info("skipping sync for secret as it is being deleted")
continue
}

if !metav1.IsControlledBy(&secret, &bundle) {
secretLog.V(2).Info("skipping sync for configmap as it is not controlled by bundle")
continue
}

targetResources[key] = false
}
}

var needsUpdate bool

for target, shouldExist := range targetResources {
targetLog := log.WithValues("target", target)
var cmSynced, secretSynced bool
var err error

if bundle.Spec.Target.ConfigMap != nil {
cmSynced, err = b.syncConfigMapTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist)
if err != nil {
targetLog.Error(err, "failed sync bundle to ConfigMap target namespace")
b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncConfigMapTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err)

b.setBundleCondition(
bundle.Status.Conditions,
&statusPatch.Conditions,
trustapi.BundleCondition{
Type: trustapi.BundleConditionSynced,
Status: metav1.ConditionFalse,
Reason: "SyncConfigMapTargetFailed",
Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err),
ObservedGeneration: bundle.Generation,
},
)

return ctrl.Result{Requeue: true}, statusPatch, nil
}
}

synced, err := b.syncTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist)
if err != nil {
targetLog.Error(err, "failed sync bundle to target namespace")
b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err)

b.setBundleCondition(
bundle.Status.Conditions,
&statusPatch.Conditions,
trustapi.BundleCondition{
Type: trustapi.BundleConditionSynced,
Status: metav1.ConditionFalse,
Reason: "SyncTargetFailed",
Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err),
ObservedGeneration: bundle.Generation,
},
)

return ctrl.Result{Requeue: true}, statusPatch, nil
if bundle.Spec.Target.Secret != nil {
secretSynced, err = b.syncSecretTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist)
if err != nil {
targetLog.Error(err, "failed sync bundle to Secret target namespace")
b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncSecretTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err)

b.setBundleCondition(
bundle.Status.Conditions,
&statusPatch.Conditions,
trustapi.BundleCondition{
Type: trustapi.BundleConditionSynced,
Status: metav1.ConditionFalse,
Reason: "SyncSecretTargetFailed",
Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err),
ObservedGeneration: bundle.Generation,
},
)

return ctrl.Result{Requeue: true}, statusPatch, nil
}
}

if synced {
if cmSynced || secretSynced {
// We need to update if any target is synced.
needsUpdate = true
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/bundle/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,19 @@ func AddBundleController(
builder.OnlyMetadata,
).

// Reconcile a Bundle on events against a Secret that it
// owns. Only cache Secret metadata.
WatchesRawSource(
source.Kind(targetCache, &corev1.Secret{}),
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
&trustapi.Bundle{},
handler.OnlyControllerOwner(),
),
builder.OnlyMetadata,
).

////// Sources //////

// Reconcile trust.cert-manager.io Bundles
Expand Down
23 changes: 23 additions & 0 deletions pkg/bundle/internal/ssa_client/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,26 @@ func GenerateConfigMapPatch(

return configmap, applyPatch{encodedPatch}, nil
}

func GenerateSecretPatch(
secretPatch *coreapplyconfig.SecretApplyConfiguration,
) (*corev1.Secret, client.Patch, error) {
if secretPatch == nil || secretPatch.Name == nil || secretPatch.Namespace == nil {
panic("secretPatch must be non-nil and have a name and namespace")
}

// This object is used to deduce the name & namespace + unmarshall the return value in
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: *secretPatch.Name,
Namespace: *secretPatch.Namespace,
},
}

encodedPatch, err := json.Marshal(secretPatch)
if err != nil {
return secret, nil, err
}

return secret, applyPatch{encodedPatch}, nil
}
Loading