From f25d31b92b36c14fc93cf3cbb47abef0a49db01c Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 22 Apr 2025 18:09:36 -0300 Subject: [PATCH] fix(code): remove dead code and reduce duplication BE-11826 (#680) --- api/archive/zip.go | 55 +++------------------------------ api/cli/pairlistbool.go | 45 --------------------------- api/docker/client/client.go | 13 -------- api/docker/images/digest.go | 11 +++---- api/docker/images/image.go | 21 ++++++++----- api/docker/images/image_test.go | 14 ++++----- api/docker/images/registry.go | 4 +-- api/kubernetes/cli/client.go | 7 ----- api/stacks/stackutils/util.go | 5 --- 9 files changed, 33 insertions(+), 142 deletions(-) delete mode 100644 api/cli/pairlistbool.go diff --git a/api/archive/zip.go b/api/archive/zip.go index 50328ef09..b7dbb9302 100644 --- a/api/archive/zip.go +++ b/api/archive/zip.go @@ -2,7 +2,6 @@ package archive import ( "archive/zip" - "bytes" "fmt" "io" "os" @@ -12,50 +11,6 @@ import ( "github.com/pkg/errors" ) -// UnzipArchive will unzip an archive from bytes into the dest destination folder on disk -func UnzipArchive(archiveData []byte, dest string) error { - zipReader, err := zip.NewReader(bytes.NewReader(archiveData), int64(len(archiveData))) - if err != nil { - return err - } - - for _, zipFile := range zipReader.File { - err := extractFileFromArchive(zipFile, dest) - if err != nil { - return err - } - } - - return nil -} - -func extractFileFromArchive(file *zip.File, dest string) error { - f, err := file.Open() - if err != nil { - return err - } - defer f.Close() - - data, err := io.ReadAll(f) - if err != nil { - return err - } - - fpath := filepath.Join(dest, file.Name) - - outFile, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode()) - if err != nil { - return err - } - - _, err = io.Copy(outFile, bytes.NewReader(data)) - if err != nil { - return err - } - - return outFile.Close() -} - // UnzipFile will decompress a zip archive, moving all files and folders // within the zip file (parameter 1) to an output directory (parameter 2). func UnzipFile(src string, dest string) error { @@ -76,11 +31,11 @@ func UnzipFile(src string, dest string) error { if f.FileInfo().IsDir() { // Make Folder os.MkdirAll(p, os.ModePerm) + continue } - err = unzipFile(f, p) - if err != nil { + if err := unzipFile(f, p); err != nil { return err } } @@ -93,20 +48,20 @@ func unzipFile(f *zip.File, p string) error { if err := os.MkdirAll(filepath.Dir(p), os.ModePerm); err != nil { return errors.Wrapf(err, "unzipFile: can't make a path %s", p) } + outFile, err := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode()) if err != nil { return errors.Wrapf(err, "unzipFile: can't create file %s", p) } defer outFile.Close() + rc, err := f.Open() if err != nil { return errors.Wrapf(err, "unzipFile: can't open zip file %s in the archive", f.Name) } defer rc.Close() - _, err = io.Copy(outFile, rc) - - if err != nil { + if _, err = io.Copy(outFile, rc); err != nil { return errors.Wrapf(err, "unzipFile: can't copy an archived file content") } diff --git a/api/cli/pairlistbool.go b/api/cli/pairlistbool.go deleted file mode 100644 index 69e89b792..000000000 --- a/api/cli/pairlistbool.go +++ /dev/null @@ -1,45 +0,0 @@ -package cli - -import ( - "strings" - - portainer "github.com/portainer/portainer/api" - - "gopkg.in/alecthomas/kingpin.v2" -) - -type pairListBool []portainer.Pair - -// Set implementation for a list of portainer.Pair -func (l *pairListBool) Set(value string) error { - p := new(portainer.Pair) - - // default to true. example setting=true is equivalent to setting - parts := strings.SplitN(value, "=", 2) - if len(parts) != 2 { - p.Name = parts[0] - p.Value = "true" - } else { - p.Name = parts[0] - p.Value = parts[1] - } - - *l = append(*l, *p) - return nil -} - -// String implementation for a list of pair -func (l *pairListBool) String() string { - return "" -} - -// IsCumulative implementation for a list of pair -func (l *pairListBool) IsCumulative() bool { - return true -} - -func BoolPairs(s kingpin.Settings) (target *[]portainer.Pair) { - target = new([]portainer.Pair) - s.SetValue((*pairListBool)(target)) - return -} diff --git a/api/docker/client/client.go b/api/docker/client/client.go index 1161c4995..d1a2a394a 100644 --- a/api/docker/client/client.go +++ b/api/docker/client/client.go @@ -73,19 +73,6 @@ func createLocalClient(endpoint *portainer.Endpoint) (*client.Client, error) { ) } -func CreateClientFromEnv() (*client.Client, error) { - return client.NewClientWithOpts( - client.FromEnv, - client.WithAPIVersionNegotiation(), - ) -} - -func CreateSimpleClient() (*client.Client, error) { - return client.NewClientWithOpts( - client.WithAPIVersionNegotiation(), - ) -} - func createTCPClient(endpoint *portainer.Endpoint, timeout *time.Duration) (*client.Client, error) { httpCli, err := httpClient(endpoint, timeout) if err != nil { diff --git a/api/docker/images/digest.go b/api/docker/images/digest.go index 591f94d1e..38638de56 100644 --- a/api/docker/images/digest.go +++ b/api/docker/images/digest.go @@ -38,10 +38,10 @@ func NewClientWithRegistry(registryClient *RegistryClient, clientFactory *docker func (c *DigestClient) RemoteDigest(image Image) (digest.Digest, error) { ctx, cancel := c.timeoutContext() defer cancel() + // Docker references with both a tag and digest are currently not supported if image.Tag != "" && image.Digest != "" { - err := image.trimDigest() - if err != nil { + if err := image.TrimDigest(); err != nil { return "", err } } @@ -69,7 +69,7 @@ func (c *DigestClient) RemoteDigest(image Image) (digest.Digest, error) { // Retrieve remote digest through HEAD request rmDigest, err := docker.GetDigest(ctx, sysCtx, rmRef) if err != nil { - // fallback to public registry for hub + // Fallback to public registry for hub if image.HubLink != "" { rmDigest, err = docker.GetDigest(ctx, c.sysCtx, rmRef) if err == nil { @@ -131,8 +131,7 @@ func ParseRepoDigests(repoDigests []string) []digest.Digest { func ParseRepoTags(repoTags []string) []*Image { images := make([]*Image, 0) for _, repoTag := range repoTags { - image := ParseRepoTag(repoTag) - if image != nil { + if image := ParseRepoTag(repoTag); image != nil { images = append(images, image) } } @@ -147,7 +146,7 @@ func ParseRepoDigest(repoDigest string) digest.Digest { d, err := digest.Parse(strings.Split(repoDigest, "@")[1]) if err != nil { - log.Warn().Msgf("Skip invalid repo digest item: %s [error: %v]", repoDigest, err) + log.Warn().Err(err).Str("digest", repoDigest).Msg("skip invalid repo item") return "" } diff --git a/api/docker/images/image.go b/api/docker/images/image.go index 55e72aff0..6b0c6eb81 100644 --- a/api/docker/images/image.go +++ b/api/docker/images/image.go @@ -26,7 +26,7 @@ type Image struct { Digest digest.Digest HubLink string named reference.Named - opts ParseImageOptions + Opts ParseImageOptions `json:"-"` } // ParseImageOptions holds image options for parsing. @@ -43,9 +43,10 @@ func (i *Image) Name() string { // FullName return the real full name may include Tag or Digest of the image, Tag first. func (i *Image) FullName() string { if i.Tag == "" { - return fmt.Sprintf("%s@%s", i.Name(), i.Digest) + return i.Name() + "@" + i.Digest.String() } - return fmt.Sprintf("%s:%s", i.Name(), i.Tag) + + return i.Name() + ":" + i.Tag } // String returns the string representation of an image, including Tag and Digest if existed. @@ -66,22 +67,25 @@ func (i *Image) Reference() string { func (i *Image) WithDigest(digest digest.Digest) (err error) { i.Digest = digest i.named, err = reference.WithDigest(i.named, digest) + return err } func (i *Image) WithTag(tag string) (err error) { i.Tag = tag i.named, err = reference.WithTag(i.named, tag) + return err } -func (i *Image) trimDigest() error { +func (i *Image) TrimDigest() error { i.Digest = "" named, err := ParseImage(ParseImageOptions{Name: i.FullName()}) if err != nil { return err } i.named = &named + return nil } @@ -92,11 +96,12 @@ func ParseImage(parseOpts ParseImageOptions) (Image, error) { if err != nil { return Image{}, errors.Wrapf(err, "parsing image %s failed", parseOpts.Name) } + // Add the latest lag if they did not provide one. named = reference.TagNameOnly(named) i := Image{ - opts: parseOpts, + Opts: parseOpts, named: named, Domain: reference.Domain(named), Path: reference.Path(named), @@ -122,15 +127,16 @@ func ParseImage(parseOpts ParseImageOptions) (Image, error) { } func (i *Image) hubLink() (string, error) { - if i.opts.HubTpl != "" { + if i.Opts.HubTpl != "" { var out bytes.Buffer tmpl, err := template.New("tmpl"). Option("missingkey=error"). - Parse(i.opts.HubTpl) + Parse(i.Opts.HubTpl) if err != nil { return "", err } err = tmpl.Execute(&out, i) + return out.String(), err } @@ -142,6 +148,7 @@ func (i *Image) hubLink() (string, error) { prefix = "_" path = strings.Replace(i.Path, "library/", "", 1) } + return "https://hub.docker.com/" + prefix + "/" + path, nil case "docker.bintray.io", "jfrog-docker-reg2.bintray.io": return "https://bintray.com/jfrog/reg2/" + strings.ReplaceAll(i.Path, "/", "%3A"), nil diff --git a/api/docker/images/image_test.go b/api/docker/images/image_test.go index 2649e387c..713a67732 100644 --- a/api/docker/images/image_test.go +++ b/api/docker/images/image_test.go @@ -16,7 +16,7 @@ func TestImageParser(t *testing.T) { }) is.NoError(err, "") is.Equal("docker.io/portainer/portainer-ee:latest", image.FullName()) - is.Equal("portainer/portainer-ee", image.opts.Name) + is.Equal("portainer/portainer-ee", image.Opts.Name) is.Equal("latest", image.Tag) is.Equal("portainer/portainer-ee", image.Path) is.Equal("docker.io", image.Domain) @@ -32,7 +32,7 @@ func TestImageParser(t *testing.T) { }) is.NoError(err, "") is.Equal("gcr.io/k8s-minikube/kicbase@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.FullName()) - is.Equal("gcr.io/k8s-minikube/kicbase@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.opts.Name) + is.Equal("gcr.io/k8s-minikube/kicbase@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.Opts.Name) is.Equal("", image.Tag) is.Equal("k8s-minikube/kicbase", image.Path) is.Equal("gcr.io", image.Domain) @@ -49,7 +49,7 @@ func TestImageParser(t *testing.T) { }) is.NoError(err, "") is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30", image.FullName()) - is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.opts.Name) + is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.Opts.Name) is.Equal("v0.0.30", image.Tag) is.Equal("k8s-minikube/kicbase", image.Path) is.Equal("gcr.io", image.Domain) @@ -71,7 +71,7 @@ func TestUpdateParsedImage(t *testing.T) { is.NoError(err, "") _ = image.WithTag("v0.0.31") is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.31", image.FullName()) - is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.opts.Name) + is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.Opts.Name) is.Equal("v0.0.31", image.Tag) is.Equal("k8s-minikube/kicbase", image.Path) is.Equal("gcr.io", image.Domain) @@ -89,7 +89,7 @@ func TestUpdateParsedImage(t *testing.T) { is.NoError(err, "") _ = image.WithDigest("sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b3") is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30", image.FullName()) - is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.opts.Name) + is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.Opts.Name) is.Equal("v0.0.30", image.Tag) is.Equal("k8s-minikube/kicbase", image.Path) is.Equal("gcr.io", image.Domain) @@ -105,9 +105,9 @@ func TestUpdateParsedImage(t *testing.T) { Name: "gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", }) is.NoError(err, "") - _ = image.trimDigest() + _ = image.TrimDigest() is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30", image.FullName()) - is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.opts.Name) + is.Equal("gcr.io/k8s-minikube/kicbase:v0.0.30@sha256:02c921df998f95e849058af14de7045efc3954d90320967418a0d1f182bbc0b2", image.Opts.Name) is.Equal("v0.0.30", image.Tag) is.Equal("k8s-minikube/kicbase", image.Path) is.Equal("gcr.io", image.Domain) diff --git a/api/docker/images/registry.go b/api/docker/images/registry.go index 10b4a6388..015385c2f 100644 --- a/api/docker/images/registry.go +++ b/api/docker/images/registry.go @@ -29,7 +29,7 @@ func (c *RegistryClient) RegistryAuth(image Image) (string, string, error) { return "", "", err } - registry, err := findBestMatchRegistry(image.opts.Name, registries) + registry, err := findBestMatchRegistry(image.Opts.Name, registries) if err != nil { return "", "", err } @@ -59,7 +59,7 @@ func (c *RegistryClient) EncodedRegistryAuth(image Image) (string, error) { return "", err } - registry, err := findBestMatchRegistry(image.opts.Name, registries) + registry, err := findBestMatchRegistry(image.Opts.Name, registries) if err != nil { return "", err } diff --git a/api/kubernetes/cli/client.go b/api/kubernetes/cli/client.go index ce76f725f..6d2cc437c 100644 --- a/api/kubernetes/cli/client.go +++ b/api/kubernetes/cli/client.go @@ -47,13 +47,6 @@ type ( } ) -func NewKubeClientFromClientset(cli *kubernetes.Clientset) *KubeClient { - return &KubeClient{ - cli: cli, - instanceID: "", - } -} - // NewClientFactory returns a new instance of a ClientFactory func NewClientFactory(signatureService portainer.DigitalSignatureService, reverseTunnelService portainer.ReverseTunnelService, dataStore dataservices.DataStore, instanceID, addrHTTPS, userSessionTimeout string) (*ClientFactory, error) { if userSessionTimeout == "" { diff --git a/api/stacks/stackutils/util.go b/api/stacks/stackutils/util.go index 5dd5bf4c5..9a4daeae3 100644 --- a/api/stacks/stackutils/util.go +++ b/api/stacks/stackutils/util.go @@ -45,11 +45,6 @@ func SanitizeLabel(value string) string { return strings.Trim(onlyAllowedCharacterString, ".-_") } -// IsGitStack checks if the stack is a git stack or not -func IsGitStack(stack *portainer.Stack) bool { - return stack.GitConfig != nil && len(stack.GitConfig.URL) != 0 -} - // IsRelativePathStack checks if the stack is a git stack or not func IsRelativePathStack(stack *portainer.Stack) bool { // Always return false in CE