From 2aeb690d370de9fee15fc7c47b66fb04a30c41d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 11 Sep 2021 22:15:23 +0200 Subject: [PATCH] Don't return a header name from auth.GetCredentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Almost every caller is using it only to wrap an error in exactly the same way, so move that error context into GetCredentials and simplify the users. (The one other caller, build, was even wrapping the error incorrectly talking about query parameters; so let it use the same text as the others.) Signed-off-by: Miloslav Trmač --- pkg/api/handlers/compat/images.go | 4 ++-- pkg/api/handlers/compat/images_build.go | 4 ++-- pkg/api/handlers/compat/images_push.go | 4 ++-- pkg/api/handlers/compat/images_search.go | 4 ++-- pkg/api/handlers/libpod/images.go | 4 ++-- pkg/api/handlers/libpod/images_pull.go | 4 ++-- pkg/api/handlers/libpod/manifests.go | 4 ++-- pkg/api/handlers/libpod/play.go | 4 ++-- pkg/auth/auth.go | 14 ++++++++++---- pkg/auth/auth_test.go | 8 +------- 10 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index 4533fddebc..c1cc99da44 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -270,9 +270,9 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { return } - authConf, authfile, key, err := auth.GetCredentials(r) + authConf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index f85df02e17..2fd5dcccdc 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -453,10 +453,10 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { } } - creds, authfile, key, err := auth.GetCredentials(r) + creds, authfile, err := auth.GetCredentials(r) if err != nil { // Credential value(s) not returned as their value is not human readable - utils.BadRequest(w, key.String(), "n/a", err) + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/compat/images_push.go b/pkg/api/handlers/compat/images_push.go index 3a84b57998..04cad204db 100644 --- a/pkg/api/handlers/compat/images_push.go +++ b/pkg/api/handlers/compat/images_push.go @@ -85,9 +85,9 @@ func PushImage(w http.ResponseWriter, r *http.Request) { return } - authconf, authfile, key, err := auth.GetCredentials(r) + authconf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "Something went wrong.", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/compat/images_search.go b/pkg/api/handlers/compat/images_search.go index e9cc3e2b63..f6ad86a044 100644 --- a/pkg/api/handlers/compat/images_search.go +++ b/pkg/api/handlers/compat/images_search.go @@ -34,9 +34,9 @@ func SearchImages(w http.ResponseWriter, r *http.Request) { return } - _, authfile, key, err := auth.GetCredentials(r) + _, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index f2f93434af..6e23845f0f 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -497,9 +497,9 @@ func PushImage(w http.ResponseWriter, r *http.Request) { return } - authconf, authfile, key, err := auth.GetCredentials(r) + authconf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/libpod/images_pull.go b/pkg/api/handlers/libpod/images_pull.go index fabdb326bf..518e7cc65a 100644 --- a/pkg/api/handlers/libpod/images_pull.go +++ b/pkg/api/handlers/libpod/images_pull.go @@ -68,9 +68,9 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { } // Do the auth dance. - authConf, authfile, key, err := auth.GetCredentials(r) + authConf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 869c83fa35..eb0b6827ff 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -176,9 +176,9 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { } source := utils.GetName(r) - authconf, authfile, key, err := auth.GetCredentials(r) + authconf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/api/handlers/libpod/play.go b/pkg/api/handlers/libpod/play.go index f943fc2402..e6ae9ad180 100644 --- a/pkg/api/handlers/libpod/play.go +++ b/pkg/api/handlers/libpod/play.go @@ -86,9 +86,9 @@ func PlayKube(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error closing temporary file")) return } - authConf, authfile, key, err := auth.GetCredentials(r) + authConf, authfile, err := auth.GetCredentials(r) if err != nil { - utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String())) + utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, err) return } defer auth.RemoveAuthfile(authfile) diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 7cde6ef5ed..c0606cf1d4 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -32,17 +32,23 @@ const XRegistryConfigHeader HeaderAuthName = "X-Registry-Config" // GetCredentials queries the http.Request for X-Registry-.* headers and extracts // the necessary authentication information for libpod operations -func GetCredentials(r *http.Request) (*types.DockerAuthConfig, string, HeaderAuthName, error) { +func GetCredentials(r *http.Request) (*types.DockerAuthConfig, string, error) { has := func(key HeaderAuthName) bool { hdr, found := r.Header[string(key)]; return found && len(hdr) > 0 } switch { case has(XRegistryConfigHeader): c, f, err := getConfigCredentials(r) - return c, f, XRegistryConfigHeader, err + if err != nil { + return nil, "", errors.Wrapf(err, "failed to parse %q header for %s", XRegistryConfigHeader, r.URL.String()) + } + return c, f, nil case has(XRegistryAuthHeader): c, f, err := getAuthCredentials(r) - return c, f, XRegistryAuthHeader, err + if err != nil { + return nil, "", errors.Wrapf(err, "failed to parse %q header for %s", XRegistryAuthHeader, r.URL.String()) + } + return c, f, nil } - return nil, "", "", nil + return nil, "", nil } // getConfigCredentials extracts one or more docker.AuthConfig from the request's diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index be86a9cbd2..634215acf0 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -104,7 +104,7 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { req.Header.Set(k, v) } - override, resPath, parsedHeader, err := GetCredentials(req) + override, resPath, err := GetCredentials(req) require.NoError(t, err, name) defer RemoveAuthfile(resPath) if tc.expectedOverride == nil { @@ -118,12 +118,6 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { require.NoError(t, err, name) assert.Equal(t, expectedAuth, auth, "%s, key %s", name, key) } - if len(headers) != 0 { - assert.Len(t, headers, 1) - assert.Equal(t, tc.headerName, parsedHeader) - } else { - assert.Equal(t, HeaderAuthName(""), parsedHeader) - } } }