Skip to content

Commit a064e4b

Browse files
kerstencamilamacedo86
kersten
and
camilamacedo86
committed
🌱 (chore): avoid 'fs' shadowing and rename 'isPluginExectuable' to 'isPluginExecutable' (kubernetes-sigs#4647)
fix: avoid 'fs' shadowing and rename 'isPluginExectuable' to 'isPluginExecutable' Renamed local 'fs' variables and parameters to 'filesystem' to prevent shadowing of the 'io/fs' package and improve code clarity. Also corrected the typo in the function name 'isPluginExectuable', now properly named 'isPluginExecutable'. This also updates corresponding test cases to follow the same naming convention. Co-authored-by: Camila Macedo <[email protected]>
1 parent 0ef2d5e commit a064e4b

File tree

2 files changed

+61
-56
lines changed

2 files changed

+61
-56
lines changed

‎pkg/cli/options.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ func WithCompletion() Option {
154154
}
155155

156156
// WithFilesystem is an Option that allows to set the filesystem used in the CLI.
157-
func WithFilesystem(fs machinery.Filesystem) Option {
157+
func WithFilesystem(filesystem machinery.Filesystem) Option {
158158
return func(c *CLI) error {
159-
if fs.FS == nil {
159+
if filesystem.FS == nil {
160160
return errors.New("invalid filesystem")
161161
}
162162

163-
c.fs = fs
163+
c.fs = filesystem
164164
return nil
165165
}
166166
}
@@ -240,14 +240,14 @@ func getPluginsRoot(host string) (pluginsRoot string, err error) {
240240

241241
// DiscoverExternalPlugins discovers the external plugins in the plugins root directory
242242
// and adds them to external.Plugin.
243-
func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
243+
func DiscoverExternalPlugins(filesystem afero.Fs) (ps []plugin.Plugin, err error) {
244244
pluginsRoot, err := retrievePluginsRoot(runtime.GOOS)
245245
if err != nil {
246246
logrus.Errorf("could not get plugins root: %v", err)
247247
return nil, err
248248
}
249249

250-
rootInfo, err := fs.Stat(pluginsRoot)
250+
rootInfo, err := filesystem.Stat(pluginsRoot)
251251
if err != nil {
252252
if errors.Is(err, afero.ErrFileNotFound) {
253253
logrus.Debugf("External plugins dir %q does not exist, skipping external plugin parsing", pluginsRoot)
@@ -260,7 +260,7 @@ func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
260260
return nil, nil
261261
}
262262

263-
pluginInfos, err := afero.ReadDir(fs, pluginsRoot)
263+
pluginInfos, err := afero.ReadDir(filesystem, pluginsRoot)
264264
if err != nil {
265265
return nil, err
266266
}
@@ -271,7 +271,7 @@ func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
271271
continue
272272
}
273273

274-
versions, err := afero.ReadDir(fs, filepath.Join(pluginsRoot, pluginInfo.Name()))
274+
versions, err := afero.ReadDir(filesystem, filepath.Join(pluginsRoot, pluginInfo.Name()))
275275
if err != nil {
276276
return nil, err
277277
}
@@ -282,7 +282,7 @@ func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
282282
continue
283283
}
284284

285-
pluginFiles, err := afero.ReadDir(fs, filepath.Join(pluginsRoot, pluginInfo.Name(), version.Name()))
285+
pluginFiles, err := afero.ReadDir(filesystem, filepath.Join(pluginsRoot, pluginInfo.Name(), version.Name()))
286286
if err != nil {
287287
return nil, err
288288
}
@@ -299,7 +299,7 @@ func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
299299

300300
if pluginFile.Name() == pluginInfo.Name() || trimmedPluginName[0] == pluginInfo.Name() {
301301
// check whether the external plugin is an executable.
302-
if !isPluginExectuable(pluginFile.Mode()) {
302+
if !isPluginExecutable(pluginFile.Mode()) {
303303
return nil, fmt.Errorf("External plugin %q found in path is not an executable", pluginFile.Name())
304304
}
305305

@@ -327,7 +327,7 @@ func DiscoverExternalPlugins(fs afero.Fs) (ps []plugin.Plugin, err error) {
327327
return ps, nil
328328
}
329329

330-
// isPluginExectuable checks if a plugin is an executable based on the bitmask and returns true or false.
331-
func isPluginExectuable(mode fs.FileMode) bool {
330+
// isPluginExecutable checks if a plugin is an executable based on the bitmask and returns true or false.
331+
func isPluginExecutable(mode fs.FileMode) bool {
332332
return mode&0o111 != 0
333333
}

‎pkg/cli/options_test.go

+50-45
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package cli
1919
import (
2020
"errors"
2121
"fmt"
22+
"io/fs"
2223
"os"
2324
"path/filepath"
2425
"runtime"
@@ -220,13 +221,15 @@ var _ = Describe("Discover external plugins", func() {
220221
pluginFilePath string
221222
pluginFileName string
222223
pluginPath string
224+
pluginsRoot string
225+
plugins []plugin.Plugin
223226
f afero.File
224-
fs machinery.Filesystem
227+
filesystem machinery.Filesystem
225228
err error
226229
)
227230

228231
BeforeEach(func() {
229-
fs = machinery.Filesystem{
232+
filesystem = machinery.Filesystem{
230233
FS: afero.NewMemMapFs(),
231234
}
232235

@@ -236,14 +239,14 @@ var _ = Describe("Discover external plugins", func() {
236239
pluginFileName = "externalPlugin.sh"
237240
pluginFilePath = filepath.Join(pluginPath, "externalPlugin", "v1", pluginFileName)
238241

239-
err = fs.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
242+
err = filesystem.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
240243
Expect(err).ToNot(HaveOccurred())
241244

242-
f, err = fs.FS.Create(pluginFilePath)
245+
f, err = filesystem.FS.Create(pluginFilePath)
243246
Expect(err).ToNot(HaveOccurred())
244247
Expect(f).ToNot(BeNil())
245248

246-
_, err = fs.FS.Stat(pluginFilePath)
249+
_, err = filesystem.FS.Stat(pluginFilePath)
247250
Expect(err).ToNot(HaveOccurred())
248251
})
249252

@@ -253,56 +256,56 @@ var _ = Describe("Discover external plugins", func() {
253256
_, err = f.WriteString(testPluginScript)
254257
Expect(err).To(Not(HaveOccurred()))
255258

256-
err = fs.FS.Chmod(pluginFilePath, filePermissions)
259+
err = filesystem.FS.Chmod(pluginFilePath, filePermissions)
257260
Expect(err).To(Not(HaveOccurred()))
258261

259-
_, err = fs.FS.Stat(pluginFilePath)
262+
_, err = filesystem.FS.Stat(pluginFilePath)
260263
Expect(err).ToNot(HaveOccurred())
261264

262-
ps, err := DiscoverExternalPlugins(fs.FS)
265+
plugins, err = DiscoverExternalPlugins(filesystem.FS)
263266
Expect(err).ToNot(HaveOccurred())
264-
Expect(ps).NotTo(BeNil())
265-
Expect(ps).To(HaveLen(1))
266-
Expect(ps[0].Name()).To(Equal("externalPlugin"))
267-
Expect(ps[0].Version().Number).To(Equal(1))
267+
Expect(plugins).NotTo(BeNil())
268+
Expect(plugins).To(HaveLen(1))
269+
Expect(plugins[0].Name()).To(Equal("externalPlugin"))
270+
Expect(plugins[0].Version().Number).To(Equal(1))
268271
})
269272

270273
It("should discover multiple external plugins and return the plugins without any errors", func() {
271274
// set the execute permissions on the first plugin executable
272-
err = fs.FS.Chmod(pluginFilePath, filePermissions)
275+
err = filesystem.FS.Chmod(pluginFilePath, filePermissions)
273276

274277
pluginFileName = "myotherexternalPlugin.sh"
275278
pluginFilePath = filepath.Join(pluginPath, "myotherexternalPlugin", "v1", pluginFileName)
276279

277-
f, err = fs.FS.Create(pluginFilePath)
280+
f, err = filesystem.FS.Create(pluginFilePath)
278281
Expect(err).ToNot(HaveOccurred())
279282
Expect(f).ToNot(BeNil())
280283

281-
_, err = fs.FS.Stat(pluginFilePath)
284+
_, err = filesystem.FS.Stat(pluginFilePath)
282285
Expect(err).ToNot(HaveOccurred())
283286

284287
_, err = f.WriteString(testPluginScript)
285288
Expect(err).To(Not(HaveOccurred()))
286289

287290
// set the execute permissions on the second plugin executable
288-
err = fs.FS.Chmod(pluginFilePath, filePermissions)
291+
err = filesystem.FS.Chmod(pluginFilePath, filePermissions)
289292
Expect(err).To(Not(HaveOccurred()))
290293

291-
_, err = fs.FS.Stat(pluginFilePath)
294+
_, err = filesystem.FS.Stat(pluginFilePath)
292295
Expect(err).ToNot(HaveOccurred())
293296

294-
ps, err := DiscoverExternalPlugins(fs.FS)
297+
plugins, err = DiscoverExternalPlugins(filesystem.FS)
295298
Expect(err).ToNot(HaveOccurred())
296-
Expect(ps).NotTo(BeNil())
297-
Expect(ps).To(HaveLen(2))
299+
Expect(plugins).NotTo(BeNil())
300+
Expect(plugins).To(HaveLen(2))
298301

299-
Expect(ps[0].Name()).To(Equal("externalPlugin"))
300-
Expect(ps[1].Name()).To(Equal("myotherexternalPlugin"))
302+
Expect(plugins[0].Name()).To(Equal("externalPlugin"))
303+
Expect(plugins[1].Name()).To(Equal("myotherexternalPlugin"))
301304
})
302305

303306
Context("that are invalid", func() {
304307
BeforeEach(func() {
305-
fs = machinery.Filesystem{
308+
filesystem = machinery.Filesystem{
306309
FS: afero.NewMemMapFs(),
307310
}
308311

@@ -314,47 +317,48 @@ var _ = Describe("Discover external plugins", func() {
314317
pluginFileName = "externalPlugin.sh"
315318
pluginFilePath = filepath.Join(pluginPath, "externalPlugin", "v1", pluginFileName)
316319

317-
err = fs.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
320+
err = filesystem.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
318321
Expect(err).ToNot(HaveOccurred())
319322

320-
f, err := fs.FS.Create(pluginFilePath)
323+
var file fs.File
324+
file, err = filesystem.FS.Create(pluginFilePath)
321325
Expect(err).ToNot(HaveOccurred())
322-
Expect(f).ToNot(BeNil())
326+
Expect(file).ToNot(BeNil())
323327

324-
_, err = fs.FS.Stat(pluginFilePath)
328+
_, err = filesystem.FS.Stat(pluginFilePath)
325329
Expect(err).ToNot(HaveOccurred())
326330

327331
// set the plugin file permissions to read-only
328-
err = fs.FS.Chmod(pluginFilePath, 0o444)
332+
err = filesystem.FS.Chmod(pluginFilePath, 0o444)
329333
Expect(err).To(Not(HaveOccurred()))
330334

331-
ps, err := DiscoverExternalPlugins(fs.FS)
335+
plugins, err = DiscoverExternalPlugins(filesystem.FS)
332336
Expect(err).To(HaveOccurred())
333337
Expect(err.Error()).To(ContainSubstring("not an executable"))
334-
Expect(ps).To(BeEmpty())
338+
Expect(plugins).To(BeEmpty())
335339
})
336340

337341
It("should error if the plugin found has an invalid plugin name", func() {
338342
pluginFileName = ".sh"
339343
pluginFilePath = filepath.Join(pluginPath, "externalPlugin", "v1", pluginFileName)
340344

341-
err = fs.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
345+
err = filesystem.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
342346
Expect(err).ToNot(HaveOccurred())
343347

344-
f, err = fs.FS.Create(pluginFilePath)
348+
f, err = filesystem.FS.Create(pluginFilePath)
345349
Expect(err).ToNot(HaveOccurred())
346350
Expect(f).ToNot(BeNil())
347351

348-
ps, err := DiscoverExternalPlugins(fs.FS)
352+
plugins, err = DiscoverExternalPlugins(filesystem.FS)
349353
Expect(err).To(HaveOccurred())
350354
Expect(err.Error()).To(ContainSubstring("Invalid plugin name found"))
351-
Expect(ps).To(BeEmpty())
355+
Expect(plugins).To(BeEmpty())
352356
})
353357
})
354358

355359
Context("that does not match the plugin root directory name", func() {
356360
BeforeEach(func() {
357-
fs = machinery.Filesystem{
361+
filesystem = machinery.Filesystem{
358362
FS: afero.NewMemMapFs(),
359363
}
360364

@@ -366,17 +370,18 @@ var _ = Describe("Discover external plugins", func() {
366370
pluginFileName = "random.sh"
367371
pluginFilePath = filepath.Join(pluginPath, "externalPlugin", "v1", pluginFileName)
368372

369-
err = fs.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
373+
err = filesystem.FS.MkdirAll(filepath.Dir(pluginFilePath), 0o700)
370374
Expect(err).ToNot(HaveOccurred())
371375

372-
f, err = fs.FS.Create(pluginFilePath)
376+
f, err = filesystem.FS.Create(pluginFilePath)
373377
Expect(err).ToNot(HaveOccurred())
374378
Expect(f).ToNot(BeNil())
375379

376-
err = fs.FS.Chmod(pluginFilePath, filePermissions)
380+
err = filesystem.FS.Chmod(pluginFilePath, filePermissions)
377381
Expect(err).ToNot(HaveOccurred())
378382

379-
ps, err := DiscoverExternalPlugins(fs.FS)
383+
var ps []plugin.Plugin
384+
ps, err = DiscoverExternalPlugins(filesystem.FS)
380385
Expect(err).ToNot(HaveOccurred())
381386
Expect(ps).To(BeEmpty())
382387
})
@@ -387,7 +392,7 @@ var _ = Describe("Discover external plugins", func() {
387392
return "", errPluginsRoot
388393
}
389394

390-
_, err := DiscoverExternalPlugins(fs.FS)
395+
_, err = DiscoverExternalPlugins(filesystem.FS)
391396
Expect(err).To(HaveOccurred())
392397

393398
Expect(err).To(Equal(errPluginsRoot))
@@ -398,7 +403,7 @@ var _ = Describe("Discover external plugins", func() {
398403
return "externalplugin.sh", nil
399404
}
400405

401-
_, err := DiscoverExternalPlugins(fs.FS)
406+
_, err = DiscoverExternalPlugins(filesystem.FS)
402407
Expect(err).ToNot(HaveOccurred())
403408
})
404409

@@ -410,7 +415,7 @@ var _ = Describe("Discover external plugins", func() {
410415

411416
home := os.Getenv("HOME")
412417

413-
pluginsRoot, err := getPluginsRoot("darwin")
418+
pluginsRoot, err = getPluginsRoot("darwin")
414419
Expect(err).ToNot(HaveOccurred())
415420
expected := filepath.Join(home, "Library", "Application Support", "kubebuilder", "plugins")
416421
Expect(pluginsRoot).To(Equal(expected))
@@ -425,7 +430,7 @@ var _ = Describe("Discover external plugins", func() {
425430
err = os.Setenv("XDG_CONFIG_HOME", "/some/random/path")
426431
Expect(err).ToNot(HaveOccurred())
427432

428-
pluginsRoot, err := getPluginsRoot(runtime.GOOS)
433+
pluginsRoot, err = getPluginsRoot(runtime.GOOS)
429434
Expect(err).ToNot(HaveOccurred())
430435
Expect(pluginsRoot).To(Equal("/some/random/path/kubebuilder/plugins"))
431436
})
@@ -443,9 +448,9 @@ var _ = Describe("Discover external plugins", func() {
443448
Expect(err).ToNot(HaveOccurred())
444449
}
445450

446-
pluginsroot, err := getPluginsRoot(runtime.GOOS)
451+
pluginsRoot, err = getPluginsRoot(runtime.GOOS)
447452
Expect(err).To(HaveOccurred())
448-
Expect(pluginsroot).To(Equal(""))
453+
Expect(pluginsRoot).To(Equal(""))
449454
Expect(err.Error()).To(ContainSubstring("error retrieving home dir"))
450455
})
451456
})

0 commit comments

Comments
 (0)