Skip to content

Commit

Permalink
refactor: Add extra linting for better line length (#42)
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmidyson authored Feb 2, 2022
1 parent b541be5 commit 7cb1b7e
Show file tree
Hide file tree
Showing 13 changed files with 215 additions and 62 deletions.
1 change: 1 addition & 0 deletions .go-tools
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
github.com/oligot/[email protected]
gotest.tools/[email protected]
github.com/segmentio/[email protected]
12 changes: 11 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ linters:
- gocritic
- gocyclo
- godot
- gofmt
- gofumpt
- gosec
- gosimple
- govet
Expand Down Expand Up @@ -54,6 +54,16 @@ linters-settings:
check-error-free-encoding: true
gci:
local-prefixes: github.com/mesosphere/mindthegap
gocritic:
enabled-tags:
- diagnostic
- experimental
- opinionated
- performance
- style
gofumpt:
lang-version: "1.17"
extra-rules: true
lll:
line-length: 120

Expand Down
31 changes: 23 additions & 8 deletions archive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ func TestArchiveDirectorySuccess(t *testing.T) {
}
var buf bytes.Buffer
_, err = io.CopyN(&buf, tr, 1024)
require.Condition(t,
func() (success bool) { return err == nil || err == io.EOF }, "error reading content from tarball")
require.Condition(
t,
func() (success bool) { return err == nil || err == io.EOF },
"error reading content from tarball",
)
archivedContents[hdr.Name] = buf.String()
}

Expand All @@ -70,24 +73,36 @@ func TestArchiveDirectorySuccess(t *testing.T) {
func TestArchiveDirectoryDestDirNotWritable(t *testing.T) {
tmpDir := t.TempDir()
notWriteable := filepath.Join(tmpDir, "notwritable")
require.NoError(t, os.Mkdir(notWriteable, 0500), "error creating not writable directory")
require.NoError(t, os.Mkdir(notWriteable, 0o500), "error creating not writable directory")
outputFile := filepath.Join(notWriteable, "out.tar.gz")
require.Error(t, archive.ArchiveDirectory("testdata", outputFile), "expected error archiving directory")
require.Error(
t,
archive.ArchiveDirectory("testdata", outputFile),
"expected error archiving directory",
)
}

func TestArchiveDirectoryDestFileExists(t *testing.T) {
tmpDir := t.TempDir()
outputFile := filepath.Join(tmpDir, "out.tar.gz")
f, err := os.OpenFile(outputFile, os.O_CREATE, 0400)
f, err := os.OpenFile(outputFile, os.O_CREATE, 0o400)
require.NoError(t, err, "error creating dummy file")
require.NoError(t, f.Close(), "error closing dummy file")
require.NoError(t, archive.ArchiveDirectory("testdata", outputFile), "unexpected error archiving directory")
require.NoError(
t,
archive.ArchiveDirectory("testdata", outputFile),
"unexpected error archiving directory",
)
}

func TestArchiveDirectoryUnreadableSource(t *testing.T) {
tmpDir := t.TempDir()
unreadable := filepath.Join(tmpDir, "unreadable")
require.NoError(t, os.Mkdir(unreadable, 0100), "error creating unreadable directory")
require.NoError(t, os.Mkdir(unreadable, 0o100), "error creating unreadable directory")
outputFile := filepath.Join(tmpDir, "out.tar.gz")
require.Error(t, archive.ArchiveDirectory(unreadable, outputFile), "expected error archiving directory")
require.Error(
t,
archive.ArchiveDirectory(unreadable, outputFile),
"expected error archiving directory",
)
}
80 changes: 62 additions & 18 deletions cmd/create/imagebundle/image_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,17 @@ func NewCommand(out output.Output) *cobra.Command {
switch {
case err == nil:
out.EndOperation(false)
return fmt.Errorf("%s already exists: specify --overwrite to overwrite existing file", outputFile)
return fmt.Errorf(
"%s already exists: specify --overwrite to overwrite existing file",
outputFile,
)
case !errors.Is(err, os.ErrNotExist):
out.EndOperation(false)
return fmt.Errorf("failed to check if output file %s already exists: %w", outputFile, err)
return fmt.Errorf(
"failed to check if output file %s already exists: %w",
outputFile,
err,
)
default:
out.EndOperation(true)
}
Expand All @@ -60,7 +67,10 @@ func NewCommand(out output.Output) *cobra.Command {
outputFileAbs, err := filepath.Abs(outputFile)
if err != nil {
out.EndOperation(false)
return fmt.Errorf("failed to determine where to create temporary directory: %w", err)
return fmt.Errorf(
"failed to determine where to create temporary directory: %w",
err,
)
}

cleaner := cleanup.NewCleaner()
Expand Down Expand Up @@ -100,7 +110,10 @@ func NewCommand(out output.Output) *cobra.Command {
if registryConfig.Credentials != nil && registryConfig.Credentials.Username != "" {
skopeoOpts = append(
skopeoOpts,
skopeo.SrcCredentials(registryConfig.Credentials.Username, registryConfig.Credentials.Password),
skopeo.SrcCredentials(
registryConfig.Credentials.Username,
registryConfig.Credentials.Password,
),
)
} else {
skopeoStdout, skopeoStderr, err := skopeoRunner.AttemptToLoginToRegistry(context.TODO(), registryName)
Expand All @@ -121,7 +134,8 @@ func NewCommand(out output.Output) *cobra.Command {
)

srcImageManifestList, skopeoStdout, skopeoStderr, err := skopeoRunner.InspectManifest(
context.TODO(), fmt.Sprintf("docker://%s", srcImageName),
context.TODO(),
fmt.Sprintf("docker://%s", srcImageName),
)
if err != nil {
out.EndOperation(false)
Expand All @@ -131,9 +145,15 @@ func NewCommand(out output.Output) *cobra.Command {
}
out.V(4).Infof("---skopeo stdout---:\n%s", skopeoStdout)
out.V(4).Infof("---skopeo stderr---:\n%s", skopeoStderr)
destImageManifestList := manifestlist.ManifestList{Versioned: srcImageManifestList.Versioned}
platformManifests := make(map[string]manifestlist.ManifestDescriptor, len(srcImageManifestList.Manifests))
for _, m := range srcImageManifestList.Manifests {
destImageManifestList := manifestlist.ManifestList{
Versioned: srcImageManifestList.Versioned,
}
platformManifests := make(
map[string]manifestlist.ManifestDescriptor,
len(srcImageManifestList.Manifests),
)
for i := range srcImageManifestList.Manifests {
m := srcImageManifestList.Manifests[i]
srcManifestPlatform := m.Platform.OS + "/" + m.Platform.Architecture
if m.Platform.Variant != "" {
srcManifestPlatform += "/" + m.Platform.Variant
Expand All @@ -150,16 +170,34 @@ func NewCommand(out output.Output) *cobra.Command {
platformManifest, ok = platformManifests[p.String()]
if !ok {
out.EndOperation(false)
return fmt.Errorf("could not find platform %s for image %s", p, srcImageName)
return fmt.Errorf(
"could not find platform %s for image %s",
p,
srcImageName,
)
}
}

skopeoStdout, skopeoStderr, err := skopeoRunner.Copy(context.TODO(),
fmt.Sprintf("docker://%s/%s@%s", registryName, imageName, platformManifest.Digest),
fmt.Sprintf("docker://%s/%s@%s", reg.Address(), imageName, platformManifest.Digest),
skopeoStdout, skopeoStderr, err := skopeoRunner.Copy(
context.TODO(),
fmt.Sprintf(
"docker://%s/%s@%s",
registryName,
imageName,
platformManifest.Digest,
),
fmt.Sprintf(
"docker://%s/%s@%s",
reg.Address(),
imageName,
platformManifest.Digest,
),
append(
skopeoOpts,
skopeo.DisableDestTLSVerify(), skopeo.OS(p.os), skopeo.Arch(p.arch), skopeo.Variant(p.variant),
skopeo.DisableDestTLSVerify(),
skopeo.OS(p.os),
skopeo.Arch(p.arch),
skopeo.Variant(p.variant),
)...,
)
if err != nil {
Expand All @@ -171,7 +209,10 @@ func NewCommand(out output.Output) *cobra.Command {
out.V(4).Infof("---skopeo stdout---:\n%s", skopeoStdout)
out.V(4).Infof("---skopeo stderr---:\n%s", skopeoStderr)

destImageManifestList.Manifests = append(destImageManifestList.Manifests, platformManifest)
destImageManifestList.Manifests = append(
destImageManifestList.Manifests,
platformManifest,
)
}
skopeoStdout, skopeoStderr, err = skopeoRunner.CopyManifest(context.TODO(),
destImageManifestList,
Expand Down Expand Up @@ -212,10 +253,13 @@ func NewCommand(out output.Output) *cobra.Command {
cmd.Flags().StringVar(&configFile, "images-file", "",
"File containing list of images to create bundle from, either as YAML configuration or a simple list of images")
_ = cmd.MarkFlagRequired("images-file")
cmd.Flags().Var(newPlatformSlicesValue([]platform{{os: "linux", arch: "amd64"}}, &platforms), "platform",
"platforms to download images (required format: <os>/<arch>[/<variant>])")
cmd.Flags().StringVar(&outputFile, "output-file", "images.tar.gz", "Output file to write image bundle to")
cmd.Flags().BoolVar(&overwrite, "overwrite", false, "Overwrite image bundle file if it already exists")
cmd.Flags().
Var(newPlatformSlicesValue([]platform{{os: "linux", arch: "amd64"}}, &platforms), "platform",
"platforms to download images (required format: <os>/<arch>[/<variant>])")
cmd.Flags().
StringVar(&outputFile, "output-file", "images.tar.gz", "Output file to write image bundle to")
cmd.Flags().
BoolVar(&overwrite, "overwrite", false, "Overwrite image bundle file if it already exists")

return cmd
}
5 changes: 4 additions & 1 deletion cmd/create/imagebundle/platforms_flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ func writePlatformsAsCSV(vals []platform) (string, error) {
func parsePlatformString(s string) (platform, error) {
splitVal := strings.Split(s, "/")
if len(splitVal) < 2 || len(splitVal) > 3 {
return platform{}, fmt.Errorf("invalid platform specification: %s (required format: <os>/<arch>[/<variant>]", s)
return platform{}, fmt.Errorf(
"invalid platform specification: %s (required format: <os>/<arch>[/<variant>]",
s,
)
}
p := platform{os: splitVal[0], arch: splitVal[1]}
if len(splitVal) == 3 {
Expand Down
19 changes: 16 additions & 3 deletions cmd/create/imagebundle/platforms_flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,11 @@ func TestSSWithComma(t *testing.T) {
values := f.Lookup("ps").Value.(*platformSliceValue)

if len(expected) != len(*values.value) {
t.Fatalf("expected number of values to be %d but got: %d", len(expected), len(*values.value))
t.Fatalf(
"expected number of values to be %d but got: %d",
len(expected),
len(*values.value),
)
}
for i, v := range *values.value {
if expected[i] != v {
Expand All @@ -256,7 +260,12 @@ func TestPSAsSliceValue(t *testing.T) {
}
})
expectedPlatform := platform{os: "windows", arch: "arm", variant: "v7"}
require.ElementsMatch(t, []platform{expectedPlatform}, ps, "Expected ps to be overwritten with 'windows/arm/v7'")
require.ElementsMatch(
t,
[]platform{expectedPlatform},
ps,
"Expected ps to be overwritten with 'windows/arm/v7'",
)
}

func TestPSGetSlice(t *testing.T) {
Expand Down Expand Up @@ -286,7 +295,11 @@ func TestPSAppend(t *testing.T) {
arg2 := fmt.Sprintf(argfmt, in[1])
require.NoError(t, f.Parse([]string{arg1, arg2}), "error parsing flags")

require.NoError(t, f.Lookup("ps").Value.(pflag.SliceValue).Append("windows/i386"), "error appending to platforms")
require.NoError(
t,
f.Lookup("ps").Value.(pflag.SliceValue).Append("windows/i386"),
"error appending to platforms",
)
require.ElementsMatch(t,
append(in, "windows/i386"),
f.Lookup("ps").Value.(pflag.SliceValue).GetSlice(),
Expand Down
7 changes: 5 additions & 2 deletions cmd/importcmd/imagebundle/image_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ func NewCommand(out output.Output) *cobra.Command {
out.EndOperation(true)

out.StartOperation("Starting temporary Docker registry")
reg, err := registry.NewRegistry(registry.Config{StorageDirectory: tempDir, ReadOnly: true})
reg, err := registry.NewRegistry(
registry.Config{StorageDirectory: tempDir, ReadOnly: true},
)
if err != nil {
out.EndOperation(false)
return fmt.Errorf("failed to create local Docker registry: %w", err)
Expand Down Expand Up @@ -95,7 +97,8 @@ func NewCommand(out output.Output) *cobra.Command {
},
}

cmd.Flags().StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
cmd.Flags().
StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
_ = cmd.MarkFlagRequired("image-bundle")
cmd.Flags().StringVar(&containerdNamespace, "containerd-namespace", "k8s.io",
"Containerd namespace to import images into")
Expand Down
12 changes: 9 additions & 3 deletions cmd/push/imagebundle/image_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ func NewCommand(out output.Output) *cobra.Command {
out.EndOperation(true)

out.StartOperation("Starting temporary Docker registry")
reg, err := registry.NewRegistry(registry.Config{StorageDirectory: tempDir, ReadOnly: true})
reg, err := registry.NewRegistry(
registry.Config{StorageDirectory: tempDir, ReadOnly: true},
)
if err != nil {
out.EndOperation(false)
return fmt.Errorf("failed to create local Docker registry: %w", err)
Expand All @@ -75,7 +77,10 @@ func NewCommand(out output.Output) *cobra.Command {
skopeoRunner, skopeoCleanup := skopeo.NewRunner()
cleaner.AddCleanupFn(func() { _ = skopeoCleanup() })

skopeoStdout, skopeoStderr, err := skopeoRunner.AttemptToLoginToRegistry(context.TODO(), destRegistry)
skopeoStdout, skopeoStderr, err := skopeoRunner.AttemptToLoginToRegistry(
context.TODO(),
destRegistry,
)
if err != nil {
out.Infof("---skopeo stdout---:\n%s", skopeoStdout)
out.Infof("---skopeo stderr---:\n%s", skopeoStderr)
Expand Down Expand Up @@ -119,7 +124,8 @@ func NewCommand(out output.Output) *cobra.Command {
},
}

cmd.Flags().StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
cmd.Flags().
StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
_ = cmd.MarkFlagRequired("image-bundle")
cmd.Flags().StringVar(&destRegistry, "to-registry", "", "Registry to push images to")
_ = cmd.MarkFlagRequired("to-registry")
Expand Down
6 changes: 4 additions & 2 deletions cmd/serve/imagebundle/image_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ func NewCommand(out output.Output) *cobra.Command {
},
}

cmd.Flags().StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
cmd.Flags().
StringVar(&imageBundleFile, "image-bundle", "", "Tarball containing list of images to push")
_ = cmd.MarkFlagRequired("image-bundle")
cmd.Flags().StringVar(&listenAddress, "listen-address", "localhost", "Address to list on")
cmd.Flags().Uint16Var(&listenPort, "listen-port", 0, "Port to listen on (0 means use any free port)")
cmd.Flags().
Uint16Var(&listenPort, "listen-port", 0, "Port to listen on (0 means use any free port)")
cmd.Flags().StringVar(&tlsCertificate, "tls-cert-file", "", "TLS certificate file")
cmd.Flags().StringVar(&tlsKey, "tls-private-key-file", "", "TLS private key file")

Expand Down
4 changes: 3 additions & 1 deletion config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ func TestParseFile(t *testing.T) {
if strings.HasSuffix(tt.name, "in plain text file") {
ext = "txt"
}
got, err := ParseFile(filepath.Join("testdata", strings.ReplaceAll(tt.name, " ", "_")+"."+ext))
got, err := ParseFile(
filepath.Join("testdata", strings.ReplaceAll(tt.name, " ", "_")+"."+ext),
)
if (err != nil) != tt.wantErr {
t.Errorf("ParseFile() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
10 changes: 8 additions & 2 deletions containerd/ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,21 @@ type CtrOption func() string
func ImportImage(ctx context.Context, src, tag, containerdNamespace string) ([]byte, error) {
baseArgs := []string{"-n", containerdNamespace}
//nolint:gosec // Args are fine.
cmd := exec.CommandContext(ctx, "ctr", append(baseArgs, []string{"images", "pull", "--plain-http", src}...)...)
cmd := exec.CommandContext(
ctx,
"ctr",
append(baseArgs, []string{"images", "pull", "--plain-http", src}...)...)
cmdOutput, err := cmd.CombinedOutput()
if err != nil {
return cmdOutput, fmt.Errorf("failed to pull image from temporary docker registry: %w", err)
}

if tag != "" {
//nolint:gosec // Args are fine.
cmd = exec.CommandContext(ctx, "ctr", append(baseArgs, []string{"images", "tag", "--force", src, tag}...)...)
cmd = exec.CommandContext(
ctx,
"ctr",
append(baseArgs, []string{"images", "tag", "--force", src, tag}...)...)
tagOutput, err := cmd.CombinedOutput()
cmdOutput = append(cmdOutput, tagOutput...)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions make/go.mk
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ endif

.PHONY: lint.%
lint.%: ## Runs golangci-lint for a specific module
lint.%: install-tool.go.golangci-lint; $(info $(M) running golangci-lint for $* module)
lint.%: install-tool.go.golangci-lint install-tool.go.golines; $(info $(M) linting $* module)
$(if $(filter-out root,$*),cd $* && )golines -w .
$(if $(filter-out root,$*),cd $* && )golangci-lint run --fix --config=$(GOLANGCI_CONFIG_FILE)
$(if $(filter-out root,$*),cd $* && )go fmt ./...
$(if $(filter-out root,$*),cd $* && )go fix ./...

.PHONY: mod-tidy
Expand Down
Loading

0 comments on commit 7cb1b7e

Please sign in to comment.