Skip to content

Commit d577718

Browse files
hiddecohiddeco
authored
Merge pull request #788 from fluxcd/tidy-nits
2 parents ca17176 + 9739e60 commit d577718

10 files changed

+41
-54
lines changed

‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ require (
4141
sigs.k8s.io/cli-utils v0.35.0
4242
sigs.k8s.io/controller-runtime v0.15.1
4343
sigs.k8s.io/kustomize/api v0.13.4
44+
sigs.k8s.io/kustomize/kyaml v0.14.2
4445
sigs.k8s.io/yaml v1.3.0
4546
)
4647

@@ -167,6 +168,5 @@ require (
167168
k8s.io/kubectl v0.27.3 // indirect
168169
oras.land/oras-go v1.2.3 // indirect
169170
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
170-
sigs.k8s.io/kustomize/kyaml v0.14.2 // indirect
171171
sigs.k8s.io/structured-merge-diff/v4 v4.3.0 // indirect
172172
)

‎internal/cmp/simple_unstructured.go

-1
Original file line numberDiff line numberDiff line change
@@ -106,5 +106,4 @@ func writePathString(path cmp.Path, sb *strings.Builder) {
106106
sb.WriteString(fmt.Sprintf("%v", t.String()))
107107
}
108108
}
109-
return
110109
}

‎internal/controller/helmrelease_controller.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request)
189189
}
190190

191191
// Log reconciliation duration
192-
durationMsg := fmt.Sprintf("reconciliation finished in %s", time.Now().Sub(start).String())
192+
durationMsg := fmt.Sprintf("reconciliation finished in %s", time.Since(start).String())
193193
if result.RequeueAfter > 0 {
194194
durationMsg = fmt.Sprintf("%s, next run in %s", durationMsg, result.RequeueAfter.String())
195195
}
@@ -440,7 +440,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context,
440440
// Remediate deployment failure if necessary.
441441
if !remediation.RetriesExhausted(hr) || remediation.MustRemediateLastFailure() {
442442
if util.ReleaseRevision(rel) <= releaseRevision {
443-
log.Info(fmt.Sprintf("skipping remediation, no new release revision created"))
443+
log.Info("skipping remediation, no new release revision created")
444444
} else {
445445
var remediationErr error
446446
switch remediation.GetStrategy() {
@@ -771,13 +771,13 @@ func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context,
771771
}
772772

773773
var reqs []reconcile.Request
774-
for _, hr := range list.Items {
774+
for i, hr := range list.Items {
775775
// If the HelmRelease is ready and the revision of the artifact equals to the
776776
// last attempted revision, we should not make a request for this HelmRelease
777-
if conditions.IsReady(&hr) && hc.GetArtifact().HasRevision(hr.Status.LastAttemptedRevision) {
777+
if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.LastAttemptedRevision) {
778778
continue
779779
}
780-
reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&hr)})
780+
reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
781781
}
782782
return reqs
783783
}

‎internal/controller/helmrelease_controller_chart.go

-12
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,6 @@ func (r *HelmReleaseReconciler) reconcileChart(ctx context.Context, hr *v2.HelmR
9595
return &helmChart, nil
9696
}
9797

98-
// getHelmChart retrieves the v1beta2.HelmChart for the given
99-
// v2beta1.HelmRelease using the name that is advertised in the status
100-
// object. It returns the v1beta2.HelmChart, or an error.
101-
func (r *HelmReleaseReconciler) getHelmChart(ctx context.Context, hr *v2.HelmRelease) (*sourcev1b2.HelmChart, error) {
102-
namespace, name := hr.Status.GetHelmChart()
103-
hc := &sourcev1b2.HelmChart{}
104-
if err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, hc); err != nil {
105-
return nil, err
106-
}
107-
return hc, nil
108-
}
109-
11098
// loadHelmChart attempts to download the artifact from the provided source,
11199
// loads it into a chart.Chart, and removes the downloaded artifact.
112100
// It returns the loaded chart.Chart on success, or an error.

‎internal/controller/helmrelease_controller_chart_test.go

+7-11
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,13 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) {
178178
g.Expect(v2.AddToScheme(scheme.Scheme)).To(Succeed())
179179
g.Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed())
180180

181-
var c client.Client
181+
c := fake.NewClientBuilder().WithScheme(scheme.Scheme)
182182
if tt.hc != nil {
183-
c = fake.NewFakeClientWithScheme(scheme.Scheme, tt.hc)
184-
} else {
185-
c = fake.NewFakeClientWithScheme(scheme.Scheme)
183+
c.WithObjects(tt.hc)
186184
}
187185

188186
r := &HelmReleaseReconciler{
189-
Client: c,
187+
Client: c.Build(),
190188
NoCrossNamespaceRef: tt.noCrossNamspaceRef,
191189
}
192190

@@ -203,7 +201,7 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) {
203201

204202
if tt.expectGC {
205203
objKey := client.ObjectKeyFromObject(tt.hc)
206-
err = c.Get(context.TODO(), objKey, tt.hc.DeepCopy())
204+
err = r.Get(context.TODO(), objKey, tt.hc.DeepCopy())
207205
g.Expect(apierrors.IsNotFound(err)).To(BeTrue())
208206
}
209207
})
@@ -259,15 +257,13 @@ func TestHelmReleaseReconciler_deleteHelmChart(t *testing.T) {
259257
g.Expect(v2.AddToScheme(scheme.Scheme)).To(Succeed())
260258
g.Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed())
261259

262-
var c client.Client
260+
c := fake.NewClientBuilder().WithScheme(scheme.Scheme)
263261
if tt.hc != nil {
264-
c = fake.NewFakeClientWithScheme(scheme.Scheme, tt.hc)
265-
} else {
266-
c = fake.NewFakeClientWithScheme(scheme.Scheme)
262+
c.WithObjects(tt.hc)
267263
}
268264

269265
r := &HelmReleaseReconciler{
270-
Client: c,
266+
Client: c.Build(),
271267
}
272268

273269
err := r.deleteHelmChart(context.TODO(), tt.hr)

‎internal/controller/helmrelease_controller_fuzz_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
"k8s.io/apimachinery/pkg/runtime"
31+
"sigs.k8s.io/controller-runtime/pkg/client"
3132
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3233
"sigs.k8s.io/yaml"
3334

@@ -122,7 +123,7 @@ other: values
122123
// v2.ValuesReference. Therefore a static value here suffices, and instead we just
123124
// play with the objects presence/absence.
124125
objectName := "values"
125-
resources := []runtime.Object{}
126+
var resources []client.Object
126127

127128
if createObject {
128129
resources = append(resources,
@@ -146,7 +147,7 @@ other: values
146147
},
147148
}
148149

149-
c := fake.NewFakeClientWithScheme(scheme, resources...)
150+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resources...).Build()
150151
r := &HelmReleaseReconciler{Client: c}
151152
var values *apiextensionsv1.JSON
152153
if hrValues != "" {
@@ -221,13 +222,13 @@ other: values
221222
hc.ObjectMeta.Name = hr.GetHelmChartName()
222223
hc.ObjectMeta.Namespace = hr.Spec.Chart.GetNamespace(hr.Namespace)
223224

224-
resources := []runtime.Object{
225+
resources := []client.Object{
225226
valuesConfigMap("values", map[string]string{valuesKey: configData}),
226227
valuesSecret("values", map[string][]byte{valuesKey: secretData}),
227228
&hc,
228229
}
229230

230-
c := fake.NewFakeClientWithScheme(scheme, resources...)
231+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resources...).Build()
231232
r := &HelmReleaseReconciler{
232233
Client: c,
233234
EventRecorder: &DummyRecorder{},

‎internal/controller/helmrelease_controller_test.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ func TestHelmReleaseReconciler_composeValues(t *testing.T) {
4343

4444
tests := []struct {
4545
name string
46-
resources []runtime.Object
46+
resources []client.Object
4747
references []v2.ValuesReference
4848
values string
4949
want chartutil.Values
5050
wantErr bool
5151
}{
5252
{
5353
name: "merges",
54-
resources: []runtime.Object{
54+
resources: []client.Object{
5555
valuesConfigMap("values", map[string]string{
5656
"values.yaml": `flat: value
5757
nested:
@@ -88,7 +88,7 @@ other: values
8888
},
8989
{
9090
name: "target path",
91-
resources: []runtime.Object{
91+
resources: []client.Object{
9292
valuesSecret("values", map[string][]byte{"single": []byte("value")}),
9393
},
9494
references: []v2.ValuesReference{
@@ -111,7 +111,7 @@ other: values
111111
},
112112
{
113113
name: "target path with boolean value",
114-
resources: []runtime.Object{
114+
resources: []client.Object{
115115
valuesSecret("values", map[string][]byte{"single": []byte("true")}),
116116
},
117117
references: []v2.ValuesReference{
@@ -134,7 +134,7 @@ other: values
134134
},
135135
{
136136
name: "target path with set-string behavior",
137-
resources: []runtime.Object{
137+
resources: []client.Object{
138138
valuesSecret("values", map[string][]byte{"single": []byte("\"true\"")}),
139139
},
140140
references: []v2.ValuesReference{
@@ -201,7 +201,7 @@ other: values
201201
},
202202
{
203203
name: "missing secret key",
204-
resources: []runtime.Object{
204+
resources: []client.Object{
205205
valuesSecret("values", nil),
206206
},
207207
references: []v2.ValuesReference{
@@ -215,7 +215,7 @@ other: values
215215
},
216216
{
217217
name: "missing config map key",
218-
resources: []runtime.Object{
218+
resources: []client.Object{
219219
valuesConfigMap("values", nil),
220220
},
221221
references: []v2.ValuesReference{
@@ -238,7 +238,7 @@ other: values
238238
},
239239
{
240240
name: "invalid values",
241-
resources: []runtime.Object{
241+
resources: []client.Object{
242242
valuesConfigMap("values", map[string]string{
243243
"values.yaml": `
244244
invalid`,
@@ -256,7 +256,7 @@ invalid`,
256256

257257
for _, tt := range tests {
258258
t.Run(tt.name, func(t *testing.T) {
259-
c := fake.NewFakeClientWithScheme(scheme, tt.resources...)
259+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.resources...).Build()
260260
r := &HelmReleaseReconciler{Client: c}
261261
var values *apiextensionsv1.JSON
262262
if tt.values != "" {

‎internal/runner/log_buffer_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ func TestLogBuffer_Log(t *testing.T) {
3737
var count int
3838
l := NewLogBuffer(func(format string, v ...interface{}) {
3939
count++
40-
return
4140
}, tt.size)
4241
for _, v := range tt.fill {
4342
l.Log("%s", v)

‎internal/runner/post_renderer_kustomize.go

+11-7
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ import (
2121
"encoding/json"
2222
"sync"
2323

24-
"sigs.k8s.io/kustomize/api/filesys"
2524
"sigs.k8s.io/kustomize/api/krusty"
2625
"sigs.k8s.io/kustomize/api/resmap"
2726
kustypes "sigs.k8s.io/kustomize/api/types"
27+
"sigs.k8s.io/kustomize/kyaml/filesys"
2828

2929
"github.com/fluxcd/pkg/apis/kustomize"
3030

@@ -46,8 +46,10 @@ func writeToFile(fs filesys.FileSystem, path string, content []byte) error {
4646
if err != nil {
4747
return err
4848
}
49-
helmOutput.Write(content)
50-
if err := helmOutput.Close(); err != nil {
49+
if _, err = helmOutput.Write(content); err != nil {
50+
return err
51+
}
52+
if err = helmOutput.Close(); err != nil {
5153
return err
5254
}
5355
return nil
@@ -58,8 +60,10 @@ func writeFile(fs filesys.FileSystem, path string, content *bytes.Buffer) error
5860
if err != nil {
5961
return err
6062
}
61-
content.WriteTo(helmOutput)
62-
if err := helmOutput.Close(); err != nil {
63+
if _, err = content.WriteTo(helmOutput); err != nil {
64+
return err
65+
}
66+
if err = helmOutput.Close(); err != nil {
6367
return err
6468
}
6569
return nil
@@ -119,14 +123,14 @@ func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedMa
119123
}
120124

121125
// Add JSON 6902 patches.
122-
for _, m := range k.spec.PatchesJSON6902 {
126+
for i, m := range k.spec.PatchesJSON6902 {
123127
patch, err := json.Marshal(m.Patch)
124128
if err != nil {
125129
return nil, err
126130
}
127131
cfg.PatchesJson6902 = append(cfg.PatchesJson6902, kustypes.Patch{
128132
Patch: string(patch),
129-
Target: adaptSelector(&m.Target),
133+
Target: adaptSelector(&k.spec.PatchesJSON6902[i].Target),
130134
})
131135
}
132136

‎internal/runner/runner.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
246246
res, err := r.config.KubeClient.Build(bytes.NewBuffer(obj.File.Data), false)
247247
if err != nil {
248248
r.config.Log("failed to parse CRDs from %s: %s", obj.Name, err)
249-
return errors.New(fmt.Sprintf("failed to parse CRDs from %s: %s", obj.Name, err))
249+
return fmt.Errorf("failed to parse CRDs from %s: %w", obj.Name, err)
250250
}
251251
allCrds = append(allCrds, res...)
252252
}
@@ -275,7 +275,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
275275
continue
276276
}
277277
r.config.Log("failed to create CRD %s: %s", crdName, err)
278-
return errors.New(fmt.Sprintf("failed to create CRD %s: %s", crdName, err))
278+
return fmt.Errorf("failed to create CRD %s: %w", crdName, err)
279279
} else {
280280
if rr != nil && rr.Created != nil {
281281
totalItems = append(totalItems, rr.Created...)
@@ -329,7 +329,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
329329
// Send them to Kube
330330
if rr, err := r.config.KubeClient.Update(original, allCrds, true); err != nil {
331331
r.config.Log("failed to apply CRD %s", err)
332-
return errors.New(fmt.Sprintf("failed to apply CRD %s", err))
332+
return fmt.Errorf("failed to apply CRD: %w", err)
333333
} else {
334334
if rr != nil {
335335
if rr.Created != nil {

0 commit comments

Comments
 (0)