-
Notifications
You must be signed in to change notification settings - Fork 11
ESO-186: Make the operator HTTP(s) proxy aware #62
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
apiVersion: v1 | ||
kind: ConfigMap | ||
metadata: | ||
labels: | ||
app.kubernetes.io/managed-by: kustomize | ||
app.kubernetes.io/name: external-secrets-operator | ||
config.openshift.io/inject-trusted-cabundle: "true" | ||
control-plane: controller-manager | ||
name: external-secrets-operator-trusted-ca-bundle |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
metadata: | ||
name: controller-manager | ||
namespace: system | ||
spec: | ||
template: | ||
spec: | ||
containers: | ||
- name: manager | ||
volumeMounts: | ||
- name: trusted-ca-bundle | ||
mountPath: /etc/pki/tls/certs | ||
readOnly: true | ||
volumes: | ||
chiragkyal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- name: trusted-ca-bundle | ||
configMap: | ||
name: trusted-ca-bundle | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainConfigMap name likely mismatched with the injected bundle manifest. This references ConfigMap "trusted-ca-bundle", while the PR context mentions creating "external-secrets-operator-trusted-ca-bundle". If the names differ, the volume will mount an empty/non‑existent CM at runtime. Apply this diff to align with the injected bundle name: configMap:
- name: trusted-ca-bundle
+ name: external-secrets-operator-trusted-ca-bundle Run this script to verify CM names across the repo and ensure consistency: 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== kustomization in config/manager =="
fd -t f kustomization.yaml | rg -n '^config/manager/.*' -n
rg -n -C2 'configMapGenerator|name:\s*trusted-ca-bundle|external-secrets-operator-trusted-ca-bundle' config/manager/kustomization.yaml || true
echo "== CM manifests mentioning trusted CA bundle =="
fd -H -t f '.*(trusted|ca-bundle).*\.ya?ml' | rg -n -C2 'external-secrets-operator-trusted-ca-bundle|trusted-ca-bundle' || true Length of output: 375 Align ConfigMap name to the injected bundle
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package external_secrets | ||
|
||
import ( | ||
"fmt" | ||
|
||
corev1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"sigs.k8s.io/controller-runtime/pkg/client" | ||
|
||
operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" | ||
) | ||
|
||
// ensureTrustedCABundleConfigMap creates or ensures the trusted CA bundle ConfigMap exists | ||
// in the operand namespace when proxy configuration is present. The ConfigMap is labeled | ||
// with the injection label required by the Cluster Network Operator (CNO), which watches | ||
// for this label and injects the cluster's trusted CA bundle into the ConfigMap's data. | ||
// This function ensures the correct labels are present so that CNO can manage the CA bundle | ||
// content as expected. | ||
func (r *Reconciler) ensureTrustedCABundleConfigMap(esc *operatorv1alpha1.ExternalSecretsConfig, resourceLabels map[string]string) error { | ||
proxyConfig := r.getProxyConfiguration(esc) | ||
|
||
// Only create ConfigMap if proxy is configured | ||
if proxyConfig == nil { | ||
return nil | ||
} | ||
|
||
namespace := getNamespace(esc) | ||
expectedLabels := getTrustedCABundleLabels(resourceLabels) | ||
|
||
configMap := &corev1.ConfigMap{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: trustedCABundleConfigMapName, | ||
Namespace: namespace, | ||
Labels: expectedLabels, | ||
}, | ||
} | ||
|
||
// Check if the ConfigMap already exists | ||
existingConfigMap := &corev1.ConfigMap{} | ||
exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(configMap), existingConfigMap) | ||
if err != nil { | ||
return fmt.Errorf("failed to check if trusted CA bundle ConfigMap exists: %w", err) | ||
} | ||
|
||
if !exist { | ||
// Create the ConfigMap | ||
if err := r.Create(r.ctx, configMap); err != nil { | ||
return fmt.Errorf("failed to create trusted CA bundle ConfigMap: %w", err) | ||
} | ||
return nil | ||
} | ||
|
||
// ConfigMap exists, ensure it has the correct labels | ||
// Do not update the data of the ConfigMap since it is managed by CNO | ||
if existingConfigMap.Labels == nil { | ||
existingConfigMap.Labels = make(map[string]string) | ||
} | ||
|
||
expectedLabels = getTrustedCABundleLabels(resourceLabels) | ||
needsUpdate := false | ||
for k, expectedValue := range expectedLabels { | ||
if existingValue, exists := existingConfigMap.Labels[k]; !exists || existingValue != expectedValue { | ||
existingConfigMap.Labels[k] = expectedValue | ||
needsUpdate = true | ||
} | ||
} | ||
|
||
// Update the ConfigMap if any labels changed | ||
if needsUpdate { | ||
if err := r.Update(r.ctx, existingConfigMap); err != nil { | ||
return fmt.Errorf("failed to update trusted CA bundle ConfigMap labels: %w", err) | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// getTrustedCABundleLabels merges resource labels with the injection label | ||
func getTrustedCABundleLabels(resourceLabels map[string]string) map[string]string { | ||
labels := make(map[string]string) | ||
for k, v := range resourceLabels { | ||
labels[k] = v | ||
} | ||
labels[trustedCABundleInjectLabel] = "true" | ||
return labels | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -74,6 +74,7 @@ var ( | |||||||||||||
&corev1.Secret{}, | ||||||||||||||
&corev1.Service{}, | ||||||||||||||
&corev1.ServiceAccount{}, | ||||||||||||||
&corev1.ConfigMap{}, | ||||||||||||||
&webhook.ValidatingWebhookConfiguration{}, | ||||||||||||||
} | ||||||||||||||
) | ||||||||||||||
|
@@ -251,28 +252,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { | |||||||||||||
mapFunc := func(ctx context.Context, obj client.Object) []reconcile.Request { | ||||||||||||||
r.log.V(4).Info("received reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) | ||||||||||||||
|
||||||||||||||
objLabels := obj.GetLabels() | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is required for |
||||||||||||||
if objLabels != nil { | ||||||||||||||
if objLabels[requestEnqueueLabelKey] == requestEnqueueLabelValue { | ||||||||||||||
return []reconcile.Request{ | ||||||||||||||
{ | ||||||||||||||
NamespacedName: types.NamespacedName{ | ||||||||||||||
Name: common.ExternalSecretsConfigObjectName, | ||||||||||||||
}, | ||||||||||||||
}, | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
// Since predicate already filtered, all objects that reach here should trigger reconciliation | ||||||||||||||
return []reconcile.Request{ | ||||||||||||||
{ | ||||||||||||||
NamespacedName: types.NamespacedName{ | ||||||||||||||
Name: common.ExternalSecretsConfigObjectName, | ||||||||||||||
}, | ||||||||||||||
}, | ||||||||||||||
} | ||||||||||||||
r.log.V(4).Info("object not of interest, ignoring reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) | ||||||||||||||
return []reconcile.Request{} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// predicate function to ignore events for objects not managed by controller. | ||||||||||||||
managedResources := predicate.NewPredicateFuncs(func(object client.Object) bool { | ||||||||||||||
return object.GetLabels() != nil && object.GetLabels()[requestEnqueueLabelKey] == requestEnqueueLabelValue | ||||||||||||||
}) | ||||||||||||||
|
||||||||||||||
// predicate function for ConfigMaps to include both managed resources and trusted CA bundle ConfigMap | ||||||||||||||
configMapPredicate := predicate.NewPredicateFuncs(func(object client.Object) bool { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The trusted-ca configmap will have the |
||||||||||||||
if object.GetLabels() != nil && object.GetLabels()[requestEnqueueLabelKey] == requestEnqueueLabelValue { | ||||||||||||||
return true | ||||||||||||||
} | ||||||||||||||
return object.GetName() == trustedCABundleConfigMapName | ||||||||||||||
}) | ||||||||||||||
|
||||||||||||||
withIgnoreStatusUpdatePredicates := builder.WithPredicates(predicate.GenerationChangedPredicate{}, managedResources) | ||||||||||||||
managedResourcePredicate := builder.WithPredicates(managedResources) | ||||||||||||||
configMapResourcePredicate := builder.WithPredicates(configMapPredicate) | ||||||||||||||
|
||||||||||||||
mgrBuilder := ctrl.NewControllerManagedBy(mgr). | ||||||||||||||
For(&operatorv1alpha1.ExternalSecretsConfig{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). | ||||||||||||||
|
@@ -288,6 +293,8 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { | |||||||||||||
} | ||||||||||||||
case &corev1.Secret{}: | ||||||||||||||
mgrBuilder.WatchesMetadata(res, handler.EnqueueRequestsFromMapFunc(mapFunc), builder.WithPredicates(predicate.LabelChangedPredicate{})) | ||||||||||||||
case &corev1.ConfigMap{}: | ||||||||||||||
mgrBuilder.Watches(res, handler.EnqueueRequestsFromMapFunc(mapFunc), configMapResourcePredicate) | ||||||||||||||
Comment on lines
294
to
+297
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do something like below for current configmap usage too, and later this would need an update. Or we can just use the
Suggested change
|
||||||||||||||
default: | ||||||||||||||
mgrBuilder.Watches(res, handler.EnqueueRequestsFromMapFunc(mapFunc), managedResourcePredicate) | ||||||||||||||
} | ||||||||||||||
|
@@ -457,5 +464,6 @@ func (r *Reconciler) cleanUp(esc *operatorv1alpha1.ExternalSecretsConfig, req ct | |||||||||||||
} | ||||||||||||||
r.log.V(1).Info("removed finalizer, cleanup complete", "request", req.NamespacedName) | ||||||||||||||
|
||||||||||||||
// TODO: Cleanup trusted CA bundle ConfigMap if it exists | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Not mandatory to act. The TODO above covers all resources created for the operand installation, which would also include trusted CA ConfigMap. |
||||||||||||||
return false, nil | ||||||||||||||
} |
Uh oh!
There was an error while loading. Please reload this page.