Skip to content

Commit 7cb25b7

Browse files
bharath-b-rhbharath-b-rh
committed
ESO-396: resolved rebase conflicts
Signed-off-by: Bharath B <bhb@redhat.com>
1 parent 3976ad1 commit 7cb25b7

14 files changed

Lines changed: 263 additions & 191 deletions

‎pkg/controller/external_secrets/certificate.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,25 @@ func (r *Reconciler) createOrApplyCertificate(esc *operatorv1alpha1.ExternalSecr
6464
return common.FromClientError(err, "failed to check %s certificate resource already exists", certificateName)
6565
}
6666

67-
if exist && recon {
67+
if !exist {
68+
return r.createWithFallback(desired, resourceMetadata, certificateName, esc)
69+
}
70+
71+
if recon {
6872
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s certificate resource already exists, maybe from previous installation", certificateName)
6973
}
70-
if exist && common.HasObjectChanged(desired, fetched, &resourceMetadata) {
71-
r.log.V(1).Info("certificate has been modified, updating to desired state", "name", certificateName)
72-
common.RemoveObsoleteAnnotations(desired, resourceMetadata)
73-
if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
74-
return common.FromClientError(err, "failed to update %s certificate resource", certificateName)
75-
}
76-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "certificate resource %s reconciled back to desired state", certificateName)
77-
} else {
74+
75+
if !common.HasObjectChanged(desired, fetched, &resourceMetadata) {
7876
r.log.V(4).Info("certificate resource already exists and is in expected state", "name", certificateName)
77+
return nil
7978
}
80-
if !exist {
81-
if err := r.createWithFallback(desired, resourceMetadata, certificateName, esc); err != nil {
82-
return err
83-
}
79+
80+
r.log.V(1).Info("certificate has been modified, updating to desired state", "name", certificateName)
81+
common.RemoveObsoleteAnnotations(desired, resourceMetadata)
82+
if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
83+
return common.FromClientError(err, "failed to update %s certificate resource", certificateName)
8484
}
85+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "certificate resource %s reconciled back to desired state", certificateName)
8586

8687
return nil
8788
}

‎pkg/controller/external_secrets/certificate_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ func TestCreateOrApplyCertificates(t *testing.T) {
124124
esc.Spec.ControllerConfig.CertProvider.CertManager.IssuerRef.Name = testIssuerName
125125
esc.Spec.ControllerConfig.CertProvider.CertManager.IssuerRef.Kind = issuerKind
126126
},
127-
recon: false,
128-
wantUserConfigErr: true,
127+
recon: false,
128+
wantUserConfigErr: true,
129129
wantUserConfigNotFound: true,
130130
},
131131
{

‎pkg/controller/external_secrets/deployments.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,9 @@ func (r *Reconciler) createOrApplyDeploymentFromAsset(esc *operatorv1alpha1.Exte
8585
}
8686

8787
if !exist {
88-
if err := r.Create(r.ctx, deployment); err != nil {
89-
return common.FromClientError(err, "failed to create %s deployment resource", deploymentName)
88+
if err := r.createWithFallback(deployment, resourceMetadata, deploymentName, esc); err != nil {
89+
return err
9090
}
91-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "deployment resource %s created", deploymentName)
9291
return trustedCAErr
9392
}
9493

@@ -108,11 +107,7 @@ func (r *Reconciler) createOrApplyDeploymentFromAsset(esc *operatorv1alpha1.Exte
108107
}
109108
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "deployment resource %s updated", deploymentName)
110109

111-
if trustedCAErr != nil {
112-
return trustedCAErr
113-
}
114-
115-
return nil
110+
return trustedCAErr
116111
}
117112

118113
func (r *Reconciler) getDeploymentObject(assetName string, esc *operatorv1alpha1.ExternalSecretsConfig, resourceMetadata common.ResourceMetadata) (*appsv1.Deployment, error) {

‎pkg/controller/external_secrets/networkpolicy.go‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,7 @@ func (r *Reconciler) createOrApplyCustomNetworkPolicy(esc *operatorv1alpha1.Exte
132132
}
133133

134134
if !exists {
135-
if err := r.Create(r.ctx, networkPolicy); err != nil {
136-
return common.FromClientError(err, "failed to create network policy %s", networkPolicyName)
137-
}
138-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s created", networkPolicyName)
139-
return nil
135+
return r.createWithFallback(networkPolicy, resourceMetadata, networkPolicyName, esc)
140136
}
141137

142138
if externalSecretsConfigCreateRecon {
@@ -174,11 +170,7 @@ func (r *Reconciler) createOrApplyNetworkPolicyFromAsset(esc *operatorv1alpha1.E
174170
}
175171

176172
if !exists {
177-
if err := r.Create(r.ctx, networkPolicy); err != nil {
178-
return common.FromClientError(err, "failed to create network policy %s", networkPolicyName)
179-
}
180-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s created", networkPolicyName)
181-
return nil
173+
return r.createWithFallback(networkPolicy, resourceMetadata, networkPolicyName, esc)
182174
}
183175

184176
if externalSecretsConfigCreateRecon {
@@ -278,12 +270,9 @@ func (r *Reconciler) reconcileProxyEgressPolicy(esc *operatorv1alpha1.ExternalSe
278270
}
279271

280272
np := buildProxyEgressNetworkPolicy(r.proxyConfig, namespace, resourceMetadata)
273+
281274
if !exists {
282-
if err := r.Create(r.ctx, np); err != nil {
283-
return common.FromClientError(err, "failed to create proxy egress network policy %s", npName)
284-
}
285-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "proxy egress NetworkPolicy %s created", npName)
286-
return nil
275+
return r.createWithFallback(np, resourceMetadata, npName, esc)
287276
}
288277

289278
if recon {

‎pkg/controller/external_secrets/networkpolicy_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func TestCreateOrApplyStaticNetworkPolicies(t *testing.T) {
145145
return nil
146146
})
147147
},
148-
wantErr: "failed to create network policy external-secrets/eso-sys-deny-all-traffic: test client error",
148+
wantErr: "failed to create NetworkPolicy external-secrets/eso-sys-deny-all-traffic: test client error",
149149
},
150150
{
151151
name: "network policy exists check fails",
@@ -343,7 +343,7 @@ func TestCreateOrApplyCustomNetworkPolicies(t *testing.T) {
343343
},
344344
}
345345
},
346-
wantErr: "failed to create network policy external-secrets/eso-user-test-fail-policy: test client error",
346+
wantErr: "failed to create NetworkPolicy external-secrets/eso-user-test-fail-policy: test client error",
347347
},
348348
{
349349
name: "custom network policy updated successfully",

‎pkg/controller/external_secrets/rbacs.go‎

Lines changed: 56 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -111,24 +111,25 @@ func (r *Reconciler) createOrApplyClusterRole(esc *operatorv1alpha1.ExternalSecr
111111
return common.FromClientError(err, "failed to check %s clusterrole resource already exists", clusterRoleName)
112112
}
113113

114-
if exist && recon {
114+
if !exist {
115+
return r.createWithFallback(obj, resourceMetadata, clusterRoleName, esc)
116+
}
117+
118+
if recon {
115119
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s clusterrole resource already exists, maybe from previous installation", clusterRoleName)
116120
}
117-
if exist && common.HasObjectChanged(obj, fetched, &resourceMetadata) {
118-
r.log.V(1).Info("clusterrole has been modified, updating to desired state", "name", clusterRoleName)
119-
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
120-
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
121-
return common.FromClientError(err, "failed to update %s clusterrole resource", clusterRoleName)
122-
}
123-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrole resource %s reconciled back to desired state", clusterRoleName)
124-
} else {
121+
122+
if !common.HasObjectChanged(obj, fetched, &resourceMetadata) {
125123
r.log.V(4).Info("clusterrole resource already exists and is in expected state", "name", clusterRoleName)
124+
return nil
126125
}
127-
if !exist {
128-
if err := r.createWithFallback(obj, resourceMetadata, clusterRoleName, esc); err != nil {
129-
return err
130-
}
126+
127+
r.log.V(1).Info("clusterrole has been modified, updating to desired state", "name", clusterRoleName)
128+
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
129+
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
130+
return common.FromClientError(err, "failed to update %s clusterrole resource", clusterRoleName)
131131
}
132+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrole resource %s reconciled back to desired state", clusterRoleName)
132133

133134
return nil
134135
}
@@ -156,24 +157,25 @@ func (r *Reconciler) createOrApplyClusterRoleBinding(esc *operatorv1alpha1.Exter
156157
return common.FromClientError(err, "failed to check %s clusterrolebinding resource already exists", clusterRoleBindingName)
157158
}
158159

159-
if exist && recon {
160+
if !exist {
161+
return r.createWithFallback(obj, resourceMetadata, clusterRoleBindingName, esc)
162+
}
163+
164+
if recon {
160165
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s clusterrolebinding resource already exists, maybe from previous installation", clusterRoleBindingName)
161166
}
162-
if exist && common.HasObjectChanged(obj, fetched, &resourceMetadata) {
163-
r.log.V(1).Info("clusterrolebinding has been modified, updating to desired state", "name", clusterRoleBindingName)
164-
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
165-
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
166-
return common.FromClientError(err, "failed to update %s clusterrolebinding resource", clusterRoleBindingName)
167-
}
168-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrolebinding resource %s reconciled back to desired state", clusterRoleBindingName)
169-
} else {
167+
168+
if !common.HasObjectChanged(obj, fetched, &resourceMetadata) {
170169
r.log.V(4).Info("clusterrolebinding resource already exists and is in expected state", "name", clusterRoleBindingName)
170+
return nil
171171
}
172-
if !exist {
173-
if err := r.createWithFallback(obj, resourceMetadata, clusterRoleBindingName, esc); err != nil {
174-
return err
175-
}
172+
173+
r.log.V(1).Info("clusterrolebinding has been modified, updating to desired state", "name", clusterRoleBindingName)
174+
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
175+
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
176+
return common.FromClientError(err, "failed to update %s clusterrolebinding resource", clusterRoleBindingName)
176177
}
178+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrolebinding resource %s reconciled back to desired state", clusterRoleBindingName)
177179

178180
return nil
179181
}
@@ -199,24 +201,25 @@ func (r *Reconciler) createOrApplyRole(esc *operatorv1alpha1.ExternalSecretsConf
199201
return common.FromClientError(err, "failed to check %s role resource already exists", roleName)
200202
}
201203

202-
if exist && recon {
204+
if !exist {
205+
return r.createWithFallback(obj, resourceMetadata, roleName, esc)
206+
}
207+
208+
if recon {
203209
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s role resource already exists, maybe from previous installation", roleName)
204210
}
205-
if exist && common.HasObjectChanged(obj, fetched, &resourceMetadata) {
206-
r.log.V(1).Info("role has been modified, updating to desired state", "name", roleName)
207-
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
208-
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
209-
return common.FromClientError(err, "failed to update %s role resource", roleName)
210-
}
211-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "role resource %s reconciled back to desired state", roleName)
212-
} else {
211+
212+
if !common.HasObjectChanged(obj, fetched, &resourceMetadata) {
213213
r.log.V(4).Info("role resource already exists and is in expected state", "name", roleName)
214+
return nil
214215
}
215-
if !exist {
216-
if err := r.createWithFallback(obj, resourceMetadata, roleName, esc); err != nil {
217-
return err
218-
}
216+
217+
r.log.V(1).Info("role has been modified, updating to desired state", "name", roleName)
218+
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
219+
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
220+
return common.FromClientError(err, "failed to update %s role resource", roleName)
219221
}
222+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "role resource %s reconciled back to desired state", roleName)
220223

221224
return nil
222225
}
@@ -241,24 +244,25 @@ func (r *Reconciler) createOrApplyRoleBinding(esc *operatorv1alpha1.ExternalSecr
241244
return common.FromClientError(err, "failed to check %s rolebinding resource already exists", roleBindingName)
242245
}
243246

244-
if exist && recon {
247+
if !exist {
248+
return r.createWithFallback(obj, resourceMetadata, roleBindingName, esc)
249+
}
250+
251+
if recon {
245252
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s rolebinding resource already exists, maybe from previous installation", roleBindingName)
246253
}
247-
if exist && common.HasObjectChanged(obj, fetched, &resourceMetadata) {
248-
r.log.V(1).Info("rolebinding has been modified, updating to desired state", "name", roleBindingName)
249-
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
250-
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
251-
return common.FromClientError(err, "failed to update %s rolebinding resource", roleBindingName)
252-
}
253-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s reconciled back to desired state", roleBindingName)
254-
} else {
254+
255+
if !common.HasObjectChanged(obj, fetched, &resourceMetadata) {
255256
r.log.V(4).Info("rolebinding resource already exists and is in expected state", "name", roleBindingName)
257+
return nil
256258
}
257-
if !exist {
258-
if err := r.createWithFallback(obj, resourceMetadata, roleBindingName, esc); err != nil {
259-
return err
260-
}
259+
260+
r.log.V(1).Info("rolebinding has been modified, updating to desired state", "name", roleBindingName)
261+
common.RemoveObsoleteAnnotations(obj, resourceMetadata)
262+
if err := r.UpdateWithRetry(r.ctx, obj); err != nil {
263+
return common.FromClientError(err, "failed to update %s rolebinding resource", roleBindingName)
261264
}
265+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s reconciled back to desired state", roleBindingName)
262266

263267
return nil
264268
}

‎pkg/controller/external_secrets/secret.go‎

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"fmt"
55

66
corev1 "k8s.io/api/core/v1"
7-
apierrors "k8s.io/apimachinery/pkg/api/errors"
87
"sigs.k8s.io/controller-runtime/pkg/client"
98

109
operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1"
@@ -29,38 +28,29 @@ func (r *Reconciler) createOrApplySecret(esc *operatorv1alpha1.ExternalSecretsCo
2928
return common.FromClientError(err, "failed to check %s secret resource already exists", secretName)
3029
}
3130

32-
if exist && recon {
31+
if !exist {
32+
// NOTE: This Secret cannot use the generic createWithFallback helper because
33+
// its Data field is managed by the external-secrets cert-controller, which injects
34+
// TLS content at runtime. On AlreadyExists we use a JSON patch that touches only
35+
// metadata, leaving cert-controller-managed TLS certificates untouched.
36+
return r.createWithMetadataFallback(desired, resourceMetadata, secretName, esc)
37+
}
38+
39+
if recon {
3340
r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s secret resource already exists, maybe from previous installation", secretName)
3441
}
3542

36-
if exist && common.ObjectMetadataModified(desired, fetched, &resourceMetadata) {
37-
r.log.V(1).Info("secret has been modified, patching metadata to desired state", "name", secretName)
38-
if err := r.patchResourceMetadata(desired, resourceMetadata); err != nil {
39-
return common.FromClientError(err, "failed to patch %s secret resource metadata", secretName)
40-
}
41-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s reconciled back to desired state", secretName)
42-
} else {
43+
if !common.ObjectMetadataModified(desired, fetched, &resourceMetadata) {
4344
r.log.V(4).Info("secret resource already exists and is in expected state", "name", secretName)
45+
return nil
4446
}
4547

46-
if !exist {
47-
// NOTE: This Secret cannot use the generic createWithFallback helper because
48-
// its Data field is managed by the external-secrets cert-controller, which injects
49-
// TLS content at runtime. On AlreadyExists we use a MergePatch that touches only
50-
// metadata, leaving cert-controller-managed TLS certificates untouched.
51-
if err := r.Create(r.ctx, desired); err != nil {
52-
if !apierrors.IsAlreadyExists(err) {
53-
return common.FromClientError(err, "failed to create %s secret resource", secretName)
54-
}
55-
r.log.V(1).Info("secret exists on API server but absent from label-filtered cache, patching metadata", "name", secretName)
56-
if patchErr := r.patchResourceMetadata(desired, resourceMetadata); patchErr != nil {
57-
return common.FromClientError(patchErr, "failed to patch %s secret resource metadata", secretName)
58-
}
59-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s restored to desired state", secretName)
60-
return nil
61-
}
62-
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s created", secretName)
48+
r.log.V(1).Info("secret has been modified, patching metadata to desired state", "name", secretName)
49+
if err := r.patchResourceMetadata(desired, resourceMetadata); err != nil {
50+
return common.FromClientError(err, "failed to patch Secret %s metadata", secretName)
6351
}
52+
r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s reconciled back to desired state", secretName)
53+
6454
return nil
6555
}
6656

‎pkg/controller/external_secrets/secret_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func TestCreateOrApplySecret(t *testing.T) {
113113
return nil
114114
})
115115
},
116-
wantErr: fmt.Sprintf("failed to patch %s/%s secret resource metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
116+
wantErr: fmt.Sprintf("failed to patch Secret %s/%s metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
117117
},
118118
{
119119
// Regression test: when the managed label is removed from the Secret, the object
@@ -151,7 +151,7 @@ func TestCreateOrApplySecret(t *testing.T) {
151151
return commontest.ErrTestClient
152152
})
153153
},
154-
wantErr: fmt.Sprintf("failed to patch %s/%s secret resource metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
154+
wantErr: fmt.Sprintf("failed to patch Secret %s/%s metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
155155
},
156156
{
157157
name: "reconciliation of secret which already exists in expected state",
@@ -195,7 +195,7 @@ func TestCreateOrApplySecret(t *testing.T) {
195195
return nil
196196
})
197197
},
198-
wantErr: fmt.Sprintf("failed to create %s/%s secret resource: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
198+
wantErr: fmt.Sprintf("failed to create Secret %s/%s: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient),
199199
},
200200
{
201201
name: "successful secret creation",

0 commit comments

Comments
 (0)