Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug check nonexist authfile #4337

Merged
merged 1 commit into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/podman/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ func buildCmd(c *cliconfig.BuildValues) error {
tags = tags[1:]
}
}
if c.BudResults.Authfile != "" {
if _, err := os.Stat(c.BudResults.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.BudResults.Authfile)
}
}

pullPolicy := imagebuildah.PullNever
if c.Pull {
Expand Down
4 changes: 2 additions & 2 deletions cmd/podman/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"strings"

"github.com/containers/buildah"
buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/define"
"github.com/containers/libpod/pkg/rootless"
"github.com/containers/libpod/pkg/sysinfo"
Expand Down Expand Up @@ -112,7 +112,7 @@ func getCreateFlags(c *cliconfig.PodmanCommand) {
"Attach to STDIN, STDOUT or STDERR (default [])",
)
createFlags.String(
"authfile", shared.GetAuthFile(""),
"authfile", buildahcli.GetDefaultAuthFile(),
"Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override",
)
createFlags.String(
Expand Down
7 changes: 7 additions & 0 deletions cmd/podman/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"fmt"
"os"
"strings"

"github.com/containers/libpod/cmd/podman/cliconfig"
Expand Down Expand Up @@ -50,6 +51,12 @@ func createCmd(c *cliconfig.CreateValues) error {
defer span.Finish()
}

if c.String("authfile") != "" {
if _, err := os.Stat(c.String("authfile")); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.String("authfile"))
}
}

if err := createInit(&c.PodmanCommand); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/podman/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import (
"os"
"strings"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/image/v5/docker"
"github.com/containers/image/v5/pkg/docker/config"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/image"
"github.com/docker/docker-credential-helpers/credentials"
"github.com/pkg/errors"
Expand Down Expand Up @@ -54,7 +54,7 @@ func init() {
flags.BoolVar(&loginCommand.StdinPassword, "password-stdin", false, "Take the password from stdin")
// Disabled flags for the remote client
if !remote {
flags.StringVar(&loginCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&loginCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&loginCommand.CertDir, "cert-dir", "", "Pathname of a directory containing TLS certificates and keys used to connect to the registry")
flags.BoolVar(&loginCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
}
Expand Down
11 changes: 7 additions & 4 deletions cmd/podman/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package main
import (
"fmt"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/image/v5/docker"
"github.com/containers/image/v5/pkg/docker/config"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/image"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -40,7 +40,7 @@ func init() {
logoutCommand.SetUsageTemplate(UsageTemplate())
flags := logoutCommand.Flags()
flags.BoolVarP(&logoutCommand.All, "all", "a", false, "Remove the cached credentials for all registries in the auth file")
flags.StringVar(&logoutCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&logoutCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
markFlagHiddenForRemoteClient("authfile", flags)
}

Expand All @@ -59,7 +59,10 @@ func logoutCmd(c *cliconfig.LogoutValues) error {
server = scrubServer(args[0])
}

sc := image.GetSystemContext("", c.Authfile, false)
sc, err := shared.GetSystemContext(c.Authfile)
if err != nil {
return err
}

if c.All {
if err := config.RemoveAllAuthentication(sc); err != nil {
Expand All @@ -69,7 +72,7 @@ func logoutCmd(c *cliconfig.LogoutValues) error {
return nil
}

err := config.RemoveAuthentication(sc, server)
err = config.RemoveAuthentication(sc, server)
switch errors.Cause(err) {
case nil:
fmt.Printf("Removed login credentials for %s\n", server)
Expand Down
12 changes: 10 additions & 2 deletions cmd/podman/play_kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package main

import (
"fmt"
"os"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/pkg/adapter"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -40,7 +42,7 @@ func init() {
flags.BoolVarP(&playKubeCommand.Quiet, "quiet", "q", false, "Suppress output information when pulling images")
// Disabled flags for the remote client
if !remote {
flags.StringVar(&playKubeCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
TomSweeneyRedHat marked this conversation as resolved.
Show resolved Hide resolved
flags.StringVar(&playKubeCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&playKubeCommand.CertDir, "cert-dir", "", "`Pathname` of a directory containing TLS certificates and keys")
flags.StringVar(&playKubeCommand.SignaturePolicy, "signature-policy", "", "`Pathname` of signature policy file (not usually used)")
flags.BoolVar(&playKubeCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
Expand All @@ -57,6 +59,12 @@ func playKubeCmd(c *cliconfig.KubePlayValues) error {
return errors.New("you must supply at least one file")
}

if c.Authfile != "" {
if _, err := os.Stat(c.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.Authfile)
}
}

ctx := getContext()
runtime, err := adapter.GetRuntime(ctx, &c.PodmanCommand)
if err != nil {
Expand Down
10 changes: 8 additions & 2 deletions cmd/podman/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import (
"os"
"strings"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/image/v5/docker"
dockerarchive "github.com/containers/image/v5/docker/archive"
"github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/image"
"github.com/containers/libpod/pkg/adapter"
"github.com/containers/libpod/pkg/util"
Expand Down Expand Up @@ -60,7 +60,7 @@ func init() {
markFlagHidden(flags, "override-os")
// Disabled flags for the remote client
if !remote {
flags.StringVar(&pullCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&pullCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&pullCommand.CertDir, "cert-dir", "", "`Pathname` of a directory containing TLS certificates and keys")
flags.StringVar(&pullCommand.SignaturePolicy, "signature-policy", "", "`Pathname` of signature policy file (not usually used)")
flags.BoolVar(&pullCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
Expand Down Expand Up @@ -96,6 +96,12 @@ func pullCmd(c *cliconfig.PullValues) (retError error) {
return errors.Errorf("too many arguments. Requires exactly 1")
}

if c.Authfile != "" {
if _, err := os.Stat(c.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.Authfile)
}
}

arr := strings.SplitN(args[0], ":", 2)
if len(arr) == 2 {
if c.Bool("all-tags") {
Expand Down
10 changes: 8 additions & 2 deletions cmd/podman/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import (
"os"
"strings"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/image/v5/directory"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/image"
"github.com/containers/libpod/pkg/adapter"
"github.com/containers/libpod/pkg/util"
Expand Down Expand Up @@ -59,7 +59,7 @@ func init() {

// Disabled flags for the remote client
if !remote {
flags.StringVar(&pushCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&pushCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&pushCommand.CertDir, "cert-dir", "", "`Pathname` of a directory containing TLS certificates and keys")
flags.BoolVar(&pushCommand.Compress, "compress", false, "Compress tarball image layers when pushing to a directory using the 'dir' transport. (default is same compression type as source)")
flags.StringVar(&pushCommand.SignaturePolicy, "signature-policy", "", "`Pathname` of signature policy file (not usually used)")
Expand All @@ -74,6 +74,12 @@ func pushCmd(c *cliconfig.PushValues) error {
destName string
)

if c.Authfile != "" {
if _, err := os.Stat(c.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.Authfile)
}
}

args := c.InputArgs
if len(args) == 0 || len(args) > 2 {
return errors.New("podman push requires at least one image name, and optionally a second to specify a different destination name")
Expand Down
7 changes: 7 additions & 0 deletions cmd/podman/run.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package main

import (
"os"

"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/pkg/adapter"
"github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -45,6 +47,11 @@ func runCmd(c *cliconfig.RunValues) error {
span, _ := opentracing.StartSpanFromContext(Ctx, "runCmd")
defer span.Finish()
}
if c.String("authfile") != "" {
if _, err := os.Stat(c.String("authfile")); err != nil {
return errors.Wrapf(err, "error checking authfile path %s", c.String("authfile"))
}
}
if err := createInit(&c.PodmanCommand); err != nil {
return err
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/podman/runlabel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"strings"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/libpodruntime"
Expand Down Expand Up @@ -60,7 +61,7 @@ func init() {
flags.BoolVarP(&runlabelCommand.Quiet, "quiet", "q", false, "Suppress output information when installing images")
// Disabled flags for the remote client
if !remote {
flags.StringVar(&runlabelCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&runlabelCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&runlabelCommand.CertDir, "cert-dir", "", "`Pathname` of a directory containing TLS certificates and keys")
flags.StringVar(&runlabelCommand.SignaturePolicy, "signature-policy", "", "`Pathname` of signature policy file (not usually used)")
flags.BoolVar(&runlabelCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
Expand Down Expand Up @@ -97,6 +98,12 @@ func runlabelCmd(c *cliconfig.RunlabelValues) error {
}
defer runtime.DeferredShutdown(false)

if c.Authfile != "" {
if _, err := os.Stat(c.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.Authfile)
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be doing this in many of the commands. Should we make a little utility function in cmd/podman/utils.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should add this kind of function in buildah and vendor from there. And add check nonexist authfile in buildah, too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine by me if that works better.

args := c.InputArgs
if len(args) < 2 {
return errors.Errorf("the runlabel command requires at least 2 arguments: LABEL IMAGE")
Expand Down
11 changes: 9 additions & 2 deletions cmd/podman/search.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package main

import (
"os"
"reflect"
"strings"

buildahcli "github.com/containers/buildah/pkg/cli"
"github.com/containers/buildah/pkg/formats"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared"
"github.com/containers/libpod/libpod/image"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -45,7 +46,7 @@ func init() {
flags.BoolVar(&searchCommand.NoTrunc, "no-trunc", false, "Do not truncate the output")
// Disabled flags for the remote client
if !remote {
flags.StringVar(&searchCommand.Authfile, "authfile", shared.GetAuthFile(""), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.StringVar(&searchCommand.Authfile, "authfile", buildahcli.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
flags.BoolVar(&searchCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
}
}
Expand All @@ -65,6 +66,12 @@ func searchCmd(c *cliconfig.SearchValues) error {
return err
}

if c.Authfile != "" {
if _, err := os.Stat(c.Authfile); err != nil {
return errors.Wrapf(err, "error getting authfile %s", c.Authfile)
}
}

searchOptions := image.SearchOptions{
NoTrunc: c.NoTrunc,
Limit: c.Limit,
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/shared/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod.
ArchitectureChoice: c.String("override-arch"),
}

newImage, err := runtime.ImageRuntime().New(ctx, name, rtc.SignaturePolicyPath, GetAuthFile(c.String("authfile")), writer, &dockerRegistryOptions, image.SigningOptions{}, nil, pullType)
newImage, err := runtime.ImageRuntime().New(ctx, name, rtc.SignaturePolicyPath, c.String("authfile"), writer, &dockerRegistryOptions, image.SigningOptions{}, nil, pullType)
if err != nil {
return nil, nil, err
}
Expand Down
21 changes: 8 additions & 13 deletions cmd/podman/shared/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,19 @@ import (
"path/filepath"
"strings"

"github.com/containers/libpod/pkg/util"
"github.com/containers/image/v5/types"
"github.com/containers/libpod/libpod/image"
"github.com/google/shlex"
"github.com/pkg/errors"
)

func GetAuthFile(authfile string) string {
TomSweeneyRedHat marked this conversation as resolved.
Show resolved Hide resolved
func GetSystemContext(authfile string) (*types.SystemContext, error) {
if authfile != "" {
return authfile
}

authfile = os.Getenv("REGISTRY_AUTH_FILE")
if authfile != "" {
return authfile
}

if runtimeDir, err := util.GetRuntimeDir(); err == nil {
return filepath.Join(runtimeDir, "containers/auth.json")
if _, err := os.Stat(authfile); err != nil {
TomSweeneyRedHat marked this conversation as resolved.
Show resolved Hide resolved
return nil, errors.Wrapf(err, "error checking authfile path %s", authfile)
}
}
return ""
return image.GetSystemContext("", authfile, false), nil
}

func substituteCommand(cmd string) (string, error) {
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,11 @@ var _ = Describe("Podman create", func() {
Expect(session.ExitCode()).To((Equal(0)))
Expect(string(session.Out.Contents())).To(ContainSubstring(ALPINEARM64DIGEST))
})

It("podman create --authfile with nonexist authfile", func() {
SkipIfRemote()
session := podmanTest.PodmanNoCache([]string{"create", "--authfile", "/tmp/nonexist", "--name=foo", ALPINE})
session.WaitWithDefaultTimeout()
Expect(session).To(Not(Equal(0)))
})
TomSweeneyRedHat marked this conversation as resolved.
Show resolved Hide resolved
})
10 changes: 10 additions & 0 deletions test/e2e/login_logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ var _ = Describe("Podman login and logout", func() {
json.Unmarshal(authInfo, &info)
fmt.Println(info)

// push should fail with nonexist authfile
session = podmanTest.Podman([]string{"push", "--authfile", "/tmp/nonexist", ALPINE, testImg})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Not(Equal(0)))

session = podmanTest.Podman([]string{"push", "--authfile", authFile, ALPINE, testImg})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expand All @@ -131,6 +136,11 @@ var _ = Describe("Podman login and logout", func() {
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

// logout should fail with nonexist authfile
session = podmanTest.Podman([]string{"logout", "--authfile", "/tmp/nonexist", server})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Not(Equal(0)))

session = podmanTest.Podman([]string{"logout", "--authfile", authFile, server})
})

Expand Down
Loading