From b162d8868c0b14961347c80df834f93f0e7e82a6 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 20:13:21 +0200 Subject: Add unit tests for multiAuthHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also rename it to parseMultiAuthHeader. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index da2d9a5c5..97e7fe1ec 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -1,11 +1,14 @@ package auth import ( + "encoding/base64" "io/ioutil" + "net/http" "testing" "github.com/containers/image/v5/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAuthConfigsToAuthFile(t *testing.T) { @@ -64,3 +67,39 @@ func TestAuthConfigsToAuthFile(t *testing.T) { } } } + +func TestParseMultiAuthHeader(t *testing.T) { + for _, tc := range []struct { + input string + shouldErr bool + expected map[string]types.DockerAuthConfig + }{ + // Empty header + {input: "", expected: nil}, + // "null" + {input: "null", expected: nil}, + // Invalid JSON + {input: "@", shouldErr: true}, + // Success + { + input: base64.URLEncoding.EncodeToString([]byte( + `{"https://index.docker.io/v1/":{"username":"u1","password":"p1"},` + + `"quay.io/libpod":{"username":"u2","password":"p2"}}`)), + expected: map[string]types.DockerAuthConfig{ + "https://index.docker.io/v1/": {Username: "u1", Password: "p1"}, + "quay.io/libpod": {Username: "u2", Password: "p2"}, + }, + }, + } { + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err, tc.input) + req.Header.Set(XRegistryAuthHeader.String(), tc.input) + res, err := parseMultiAuthHeader(req) + if tc.shouldErr { + assert.Error(t, err, tc.input) + } else { + require.NoError(t, err, tc.input) + assert.Equal(t, tc.expected, res, tc.input) + } + } +} -- cgit v1.2.3-54-g00ecf From ff003928b2360304f6b9458d324df090917fab02 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 20:24:07 +0200 Subject: Add unit tests for singleAuthHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also rename it to parseSingleAuthHeader Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 6 +++--- pkg/auth/auth_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 73822eecf..60c40a40b 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -115,7 +115,7 @@ func getAuthCredentials(r *http.Request) (*types.DockerAuthConfig, string, error } // Fallback to looking for a single-auth header (i.e., one config). - authConfigs, err = singleAuthHeader(r) + authConfigs, err = parseSingleAuthHeader(r) if err != nil { return nil, "", err } @@ -309,9 +309,9 @@ func imageAuthToDockerAuth(authConfig types.DockerAuthConfig) dockerAPITypes.Aut } } -// singleAuthHeader extracts a DockerAuthConfig from the request's header. +// parseSingleAuthHeader extracts a DockerAuthConfig from the request's header. // The header content is a single DockerAuthConfig. -func singleAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, error) { +func parseSingleAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, error) { authHeader := r.Header.Get(string(XRegistryAuthHeader)) authConfig := dockerAPITypes.AuthConfig{} // Accept "null" and handle it as empty value for compatibility reason with Docker. diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 97e7fe1ec..a0b97b106 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -68,6 +68,43 @@ func TestAuthConfigsToAuthFile(t *testing.T) { } } +func TestParseSingleAuthHeader(t *testing.T) { + for _, tc := range []struct { + input string + shouldErr bool + expected map[string]types.DockerAuthConfig + }{ + { + input: "", // An empty (or missing) header + expected: map[string]types.DockerAuthConfig{"0": {}}, + }, + { + input: "null", + expected: map[string]types.DockerAuthConfig{"0": {}}, + }, + // Invalid JSON + {input: "@", shouldErr: true}, + // Success + { + input: base64.URLEncoding.EncodeToString([]byte(`{"username":"u1","password":"p1"}`)), + expected: map[string]types.DockerAuthConfig{ + "0": {Username: "u1", Password: "p1"}, + }, + }, + } { + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err, tc.input) + req.Header.Set(XRegistryAuthHeader.String(), tc.input) + res, err := parseSingleAuthHeader(req) + if tc.shouldErr { + assert.Error(t, err, tc.input) + } else { + require.NoError(t, err, tc.input) + assert.Equal(t, tc.expected, res, tc.input) + } + } +} + func TestParseMultiAuthHeader(t *testing.T) { for _, tc := range []struct { input string -- cgit v1.2.3-54-g00ecf From 5a5aa6009fdfde0524bc4e551742c366ad0164df Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 20:26:28 +0200 Subject: Improve TestAuthConfigsToAuthFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the created temporary file. Use more appropriate assertion calls. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index a0b97b106..38f82ee04 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "io/ioutil" "net/http" + "os" "testing" "github.com/containers/image/v5/types" @@ -57,13 +58,14 @@ func TestAuthConfigsToAuthFile(t *testing.T) { filePath, err := authConfigsToAuthFile(configs) if tc.shouldErr { - assert.NotNil(t, err) + assert.Error(t, err) assert.Empty(t, filePath) } else { - assert.Nil(t, err) + assert.NoError(t, err) content, err := ioutil.ReadFile(filePath) - assert.Nil(t, err) + require.NoError(t, err) assert.Contains(t, string(content), tc.expectedContains) + os.Remove(filePath) } } } -- cgit v1.2.3-54-g00ecf From ad7e5e34f20da1422f79b704fc10a3cfec85e447 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 21:13:33 +0200 Subject: Add tests for auth.Header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just a single function that handles all of Header, headerConfig and headerAuth; we will split that later. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 38f82ee04..e39a0e041 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -2,6 +2,8 @@ package auth import ( "encoding/base64" + "encoding/json" + "fmt" "io/ioutil" "net/http" "os" @@ -12,6 +14,132 @@ import ( "github.com/stretchr/testify/require" ) +const largeAuthFile = `{"auths":{ + "docker.io/vendor": {"auth": "ZG9ja2VyOnZlbmRvcg=="}, + "https://index.docker.io/v1": {"auth": "ZG9ja2VyOnRvcA=="}, + "quay.io/libpod": {"auth": "cXVheTpsaWJwb2Q="}, + "quay.io": {"auth": "cXVheTp0b3A="} +}}` + +func TestHeader(t *testing.T) { + for _, tc := range []struct { + headerName HeaderAuthName + name string + fileContents string + username, password string + shouldErr bool + expectedContents string + }{ + { + headerName: XRegistryConfigHeader, + name: "no data", + fileContents: "", + username: "", + password: "", + expectedContents: "", + }, + { + headerName: XRegistryConfigHeader, + name: "invalid JSON", + fileContents: "@invalid JSON", + username: "", + password: "", + shouldErr: true, + }, + { + headerName: XRegistryConfigHeader, + name: "file data", + fileContents: largeAuthFile, + username: "", + password: "", + expectedContents: `{ + "docker.io/vendor": {"username": "docker", "password": "vendor"}, + "docker.io": {"username": "docker", "password": "top"}, + "quay.io/libpod": {"username": "quay", "password": "libpod"}, + "quay.io": {"username": "quay", "password": "top"} + }`, + }, + { + headerName: XRegistryConfigHeader, + name: "file data + override", + fileContents: largeAuthFile, + username: "override-user", + password: "override-pass", + expectedContents: `{ + "docker.io/vendor": {"username": "docker", "password": "vendor"}, + "docker.io": {"username": "docker", "password": "top"}, + "quay.io/libpod": {"username": "quay", "password": "libpod"}, + "quay.io": {"username": "quay", "password": "top"}, + "": {"username": "override-user", "password": "override-pass"} + }`, + }, + { + headerName: XRegistryAuthHeader, + name: "override", + fileContents: "", + username: "override-user", + password: "override-pass", + expectedContents: `{"username": "override-user", "password": "override-pass"}`, + }, + { + headerName: XRegistryAuthHeader, + name: "invalid JSON", + fileContents: "@invalid JSON", + username: "", + password: "", + shouldErr: true, + }, + { + headerName: XRegistryAuthHeader, + name: "file data", + fileContents: largeAuthFile, + username: "", + password: "", + expectedContents: `{ + "docker.io/vendor": {"username": "docker", "password": "vendor"}, + "docker.io": {"username": "docker", "password": "top"}, + "quay.io/libpod": {"username": "quay", "password": "libpod"}, + "quay.io": {"username": "quay", "password": "top"} + }`, + }, + } { + name := fmt.Sprintf("%s: %s", tc.headerName, tc.name) + authFile := "" + if tc.fileContents != "" { + f, err := ioutil.TempFile("", "auth.json") + require.NoError(t, err, name) + defer os.Remove(f.Name()) + authFile = f.Name() + err = ioutil.WriteFile(authFile, []byte(tc.fileContents), 0700) + require.NoError(t, err, name) + } + + res, err := Header(nil, tc.headerName, authFile, tc.username, tc.password) + if tc.shouldErr { + assert.Error(t, err, name) + } else { + require.NoError(t, err, name) + if tc.expectedContents == "" { + assert.Empty(t, res, name) + } else { + require.Len(t, res, 1, name) + header, ok := res[tc.headerName.String()] + require.True(t, ok, name) + decodedHeader, err := base64.URLEncoding.DecodeString(header) + require.NoError(t, err, name) + // Don't test for a specific JSON representation, just for the expected contents. + expected := map[string]interface{}{} + actual := map[string]interface{}{} + err = json.Unmarshal([]byte(tc.expectedContents), &expected) + require.NoError(t, err, name) + err = json.Unmarshal(decodedHeader, &actual) + require.NoError(t, err, name) + assert.Equal(t, expected, actual, name) + } + } + } +} + func TestAuthConfigsToAuthFile(t *testing.T) { for _, tc := range []struct { name string -- cgit v1.2.3-54-g00ecf From d29a4a6d173988b83ac78355e271d0f60e5d2830 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 21:39:26 +0200 Subject: Add TestHeaderGetCredentialsRoundtrip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... as an end-to-end unit test of the header creation/parsing code. Leave the docker.io and docker.io/vendor test cases commented out, because they are currently failing. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index e39a0e041..ee16d832b 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -9,6 +9,7 @@ import ( "os" "testing" + "github.com/containers/image/v5/pkg/docker/config" "github.com/containers/image/v5/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,6 +22,111 @@ const largeAuthFile = `{"auths":{ "quay.io": {"auth": "cXVheTp0b3A="} }}` +// Semantics of largeAuthFile +var largeAuthFileValues = map[string]types.DockerAuthConfig{ + // "docker.io/vendor": {Username: "docker", Password: "vendor"}, + // "docker.io": {Username: "docker", Password: "top"}, + "quay.io/libpod": {Username: "quay", Password: "libpod"}, + "quay.io": {Username: "quay", Password: "top"}, +} + +// Test that GetCredentials() correctly parses what Header() produces +func TestHeaderGetCredentialsRoundtrip(t *testing.T) { + for _, tc := range []struct { + headerName HeaderAuthName + name string + fileContents string + username, password string + expectedOverride *types.DockerAuthConfig + expectedFileValues map[string]types.DockerAuthConfig + }{ + { + headerName: XRegistryConfigHeader, + name: "no data", + fileContents: "", + username: "", + password: "", + expectedOverride: nil, + expectedFileValues: nil, + }, + { + headerName: XRegistryConfigHeader, + name: "file data", + fileContents: largeAuthFile, + username: "", + password: "", + expectedOverride: nil, + expectedFileValues: largeAuthFileValues, + }, + { + headerName: XRegistryConfigHeader, + name: "file data + override", + fileContents: largeAuthFile, + username: "override-user", + password: "override-pass", + expectedOverride: &types.DockerAuthConfig{Username: "override-user", Password: "override-pass"}, + expectedFileValues: largeAuthFileValues, + }, + { + headerName: XRegistryAuthHeader, + name: "override", + fileContents: "", + username: "override-user", + password: "override-pass", + expectedOverride: &types.DockerAuthConfig{Username: "override-user", Password: "override-pass"}, + expectedFileValues: nil, + }, + { + headerName: XRegistryAuthHeader, + name: "file data", + fileContents: largeAuthFile, + username: "", + password: "", + expectedFileValues: largeAuthFileValues, + }, + } { + name := fmt.Sprintf("%s: %s", tc.headerName, tc.name) + inputAuthFile := "" + if tc.fileContents != "" { + f, err := ioutil.TempFile("", "auth.json") + require.NoError(t, err, name) + defer os.Remove(f.Name()) + inputAuthFile = f.Name() + err = ioutil.WriteFile(inputAuthFile, []byte(tc.fileContents), 0700) + require.NoError(t, err, name) + } + + headers, err := Header(nil, tc.headerName, inputAuthFile, tc.username, tc.password) + require.NoError(t, err) + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err, name) + for k, v := range headers { + req.Header.Set(k, v) + } + + override, resPath, parsedHeader, err := GetCredentials(req) + require.NoError(t, err, name) + defer RemoveAuthfile(resPath) + if tc.expectedOverride == nil { + assert.Nil(t, override, name) + } else { + require.NotNil(t, override, name) + assert.Equal(t, *tc.expectedOverride, *override, name) + } + for key, expectedAuth := range tc.expectedFileValues { + auth, err := config.GetCredentials(&types.SystemContext{AuthFilePath: resPath}, key) + 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) + } + } +} + func TestHeader(t *testing.T) { for _, tc := range []struct { headerName HeaderAuthName -- cgit v1.2.3-54-g00ecf From 491951d66e1829ad8e847f3049a557dd9d55db68 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 21:56:40 +0200 Subject: Fix normalizeAuthFileKey to use the correct semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 18 +++++++++++------- pkg/auth/auth_test.go | 24 ++++++++++++------------ 2 files changed, 23 insertions(+), 19 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 8c6436883..7cde6ef5e 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -272,20 +272,24 @@ func authConfigsToAuthFile(authConfigs map[string]types.DockerAuthConfig) (strin return authFilePath, nil } -// normalizeAuthFileKey takes an auth file key and removes the leading "http[s]://" prefix as well -// as removes path suffixes from docker registries. +// normalizeAuthFileKey takes an auth file key and converts it into a new-style credential key +// in the canonical format, as interpreted by c/image/pkg/docker/config. func normalizeAuthFileKey(authFileKey string) string { stripped := strings.TrimPrefix(authFileKey, "http://") stripped = strings.TrimPrefix(stripped, "https://") - /// Normalize docker registries - if strings.HasPrefix(stripped, "index.docker.io/") || - strings.HasPrefix(stripped, "registry-1.docker.io/") || - strings.HasPrefix(stripped, "docker.io/") { + if stripped != authFileKey { // URLs are interpreted to mean complete registries stripped = strings.SplitN(stripped, "/", 2)[0] } - return stripped + // Only non-namespaced registry names (or URLs) need to be normalized; repo namespaces + // always use the simple format. + switch stripped { + case "registry-1.docker.io", "index.docker.io": + return "docker.io" + default: + return stripped + } } // dockerAuthToImageAuth converts a docker auth config to one we're using diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index ee16d832b..be86a9cbd 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -24,10 +24,10 @@ const largeAuthFile = `{"auths":{ // Semantics of largeAuthFile var largeAuthFileValues = map[string]types.DockerAuthConfig{ - // "docker.io/vendor": {Username: "docker", Password: "vendor"}, - // "docker.io": {Username: "docker", Password: "top"}, - "quay.io/libpod": {Username: "quay", Password: "libpod"}, - "quay.io": {Username: "quay", Password: "top"}, + "docker.io/vendor": {Username: "docker", Password: "vendor"}, + "docker.io": {Username: "docker", Password: "top"}, + "quay.io/libpod": {Username: "quay", Password: "libpod"}, + "quay.io": {Username: "quay", Password: "top"}, } // Test that GetCredentials() correctly parses what Header() produces @@ -260,28 +260,28 @@ func TestAuthConfigsToAuthFile(t *testing.T) { expectedContains: "{}", }, { - name: "registry with prefix", + name: "registry with a namespace prefix", server: "my-registry.local/username", shouldErr: false, expectedContains: `"my-registry.local/username":`, }, { - name: "normalize https:// prefix", + name: "URLs are interpreted as full registries", server: "http://my-registry.local/username", shouldErr: false, - expectedContains: `"my-registry.local/username":`, + expectedContains: `"my-registry.local":`, }, { - name: "normalize docker registry with https prefix", + name: "the old-style docker registry URL is normalized", server: "http://index.docker.io/v1/", shouldErr: false, - expectedContains: `"index.docker.io":`, + expectedContains: `"docker.io":`, }, { - name: "normalize docker registry without https prefix", - server: "docker.io/v2/", + name: "docker.io vendor namespace", + server: "docker.io/vendor", shouldErr: false, - expectedContains: `"docker.io":`, + expectedContains: `"docker.io/vendor":`, }, } { configs := map[string]types.DockerAuthConfig{} -- cgit v1.2.3-54-g00ecf From 2aeb690d370de9fee15fc7c47b66fb04a30c41d8 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 22:15:23 +0200 Subject: 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(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index 4533fddeb..c1cc99da4 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 f85df02e1..2fd5dcccd 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 3a84b5799..04cad204d 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 e9cc3e2b6..f6ad86a04 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 f2f93434a..6e23845f0 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 fabdb326b..518e7cc65 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 869c83fa3..eb0b6827f 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 f943fc240..e6ae9ad18 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 7cde6ef5e..c0606cf1d 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 be86a9cbd..634215acf 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) - } } } -- cgit v1.2.3-54-g00ecf From 7674f2f76b07058aa3bbf44675b7c2482c61811a Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 22:20:26 +0200 Subject: Simplify the interface of parseSingleAuthHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't create a single-element map only for the only caller to laboriously extract an element of that map; just return a single entry. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 18 +++++------------- pkg/auth/auth_test.go | 12 +++++------- 2 files changed, 10 insertions(+), 20 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index c0606cf1d..c19151c91 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -121,17 +121,11 @@ func getAuthCredentials(r *http.Request) (*types.DockerAuthConfig, string, error } // Fallback to looking for a single-auth header (i.e., one config). - authConfigs, err = parseSingleAuthHeader(r) + authConfig, err := parseSingleAuthHeader(r) if err != nil { return nil, "", err } - var conf *types.DockerAuthConfig - for k := range authConfigs { - c := authConfigs[k] - conf = &c - break - } - return conf, "", nil + return &authConfig, "", nil } // Header builds the requested Authentication Header @@ -321,7 +315,7 @@ func imageAuthToDockerAuth(authConfig types.DockerAuthConfig) dockerAPITypes.Aut // parseSingleAuthHeader extracts a DockerAuthConfig from the request's header. // The header content is a single DockerAuthConfig. -func parseSingleAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, error) { +func parseSingleAuthHeader(r *http.Request) (types.DockerAuthConfig, error) { authHeader := r.Header.Get(string(XRegistryAuthHeader)) authConfig := dockerAPITypes.AuthConfig{} // Accept "null" and handle it as empty value for compatibility reason with Docker. @@ -329,12 +323,10 @@ func parseSingleAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, if len(authHeader) > 0 && authHeader != "null" { authJSON := base64.NewDecoder(base64.URLEncoding, strings.NewReader(authHeader)) if err := json.NewDecoder(authJSON).Decode(&authConfig); err != nil { - return nil, err + return types.DockerAuthConfig{}, err } } - authConfigs := make(map[string]types.DockerAuthConfig) - authConfigs["0"] = dockerAuthToImageAuth(authConfig) - return authConfigs, nil + return dockerAuthToImageAuth(authConfig), nil } // parseMultiAuthHeader extracts a DockerAuthConfig from the request's header. diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 634215acf..0e6bd42ef 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -302,24 +302,22 @@ func TestParseSingleAuthHeader(t *testing.T) { for _, tc := range []struct { input string shouldErr bool - expected map[string]types.DockerAuthConfig + expected types.DockerAuthConfig }{ { input: "", // An empty (or missing) header - expected: map[string]types.DockerAuthConfig{"0": {}}, + expected: types.DockerAuthConfig{}, }, { input: "null", - expected: map[string]types.DockerAuthConfig{"0": {}}, + expected: types.DockerAuthConfig{}, }, // Invalid JSON {input: "@", shouldErr: true}, // Success { - input: base64.URLEncoding.EncodeToString([]byte(`{"username":"u1","password":"p1"}`)), - expected: map[string]types.DockerAuthConfig{ - "0": {Username: "u1", Password: "p1"}, - }, + input: base64.URLEncoding.EncodeToString([]byte(`{"username":"u1","password":"p1"}`)), + expected: types.DockerAuthConfig{Username: "u1", Password: "p1"}, }, } { req, err := http.NewRequest(http.MethodPost, "/", nil) -- cgit v1.2.3-54-g00ecf From 1ecc6ba72852700f748715124bdf98573cc93c0b Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Sat, 11 Sep 2021 22:28:24 +0200 Subject: Pass a header value directly to parseSingleAuthHeader and parseMultiAuthHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both have a single caller, so there's no point in looking up the header value twice. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 16 ++++++++-------- pkg/auth/auth_test.go | 10 ++-------- 2 files changed, 10 insertions(+), 16 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index b3fd71184..3ecdd99fe 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -113,15 +113,17 @@ func getConfigCredentials(r *http.Request) (*types.DockerAuthConfig, string, err // stored in a temporary auth file (2nd return value). Note that the auth file // should be removed after usage. func getAuthCredentials(r *http.Request) (*types.DockerAuthConfig, string, error) { + authHeader := r.Header.Get(XRegistryAuthHeader.String()) + // First look for a multi-auth header (i.e., a map). - authConfigs, err := parseMultiAuthHeader(r) + authConfigs, err := parseMultiAuthHeader(authHeader) if err == nil { authfile, err := authConfigsToAuthFile(authConfigs) return nil, authfile, err } // Fallback to looking for a single-auth header (i.e., one config). - authConfig, err := parseSingleAuthHeader(r) + authConfig, err := parseSingleAuthHeader(authHeader) if err != nil { return nil, "", err } @@ -313,10 +315,9 @@ func imageAuthToDockerAuth(authConfig types.DockerAuthConfig) dockerAPITypes.Aut } } -// parseSingleAuthHeader extracts a DockerAuthConfig from the request's header. +// parseSingleAuthHeader extracts a DockerAuthConfig from an XRegistryAuthHeader value. // The header content is a single DockerAuthConfig. -func parseSingleAuthHeader(r *http.Request) (types.DockerAuthConfig, error) { - authHeader := r.Header.Get(string(XRegistryAuthHeader)) +func parseSingleAuthHeader(authHeader string) (types.DockerAuthConfig, error) { // Accept "null" and handle it as empty value for compatibility reason with Docker. // Some java docker clients pass this value, e.g. this one used in Eclipse. if len(authHeader) == 0 || authHeader == "null" { @@ -331,10 +332,9 @@ func parseSingleAuthHeader(r *http.Request) (types.DockerAuthConfig, error) { return dockerAuthToImageAuth(authConfig), nil } -// parseMultiAuthHeader extracts a DockerAuthConfig from the request's header. +// parseMultiAuthHeader extracts a DockerAuthConfig from an XRegistryAuthHeader value. // The header content is a map[string]DockerAuthConfigs. -func parseMultiAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, error) { - authHeader := r.Header.Get(string(XRegistryAuthHeader)) +func parseMultiAuthHeader(authHeader string) (map[string]types.DockerAuthConfig, error) { // Accept "null" and handle it as empty value for compatibility reason with Docker. // Some java docker clients pass this value, e.g. this one used in Eclipse. if len(authHeader) == 0 || authHeader == "null" { diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 0e6bd42ef..6acf1f8fb 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -320,10 +320,7 @@ func TestParseSingleAuthHeader(t *testing.T) { expected: types.DockerAuthConfig{Username: "u1", Password: "p1"}, }, } { - req, err := http.NewRequest(http.MethodPost, "/", nil) - require.NoError(t, err, tc.input) - req.Header.Set(XRegistryAuthHeader.String(), tc.input) - res, err := parseSingleAuthHeader(req) + res, err := parseSingleAuthHeader(tc.input) if tc.shouldErr { assert.Error(t, err, tc.input) } else { @@ -356,10 +353,7 @@ func TestParseMultiAuthHeader(t *testing.T) { }, }, } { - req, err := http.NewRequest(http.MethodPost, "/", nil) - require.NoError(t, err, tc.input) - req.Header.Set(XRegistryAuthHeader.String(), tc.input) - res, err := parseMultiAuthHeader(req) + res, err := parseMultiAuthHeader(tc.input) if tc.shouldErr { assert.Error(t, err, tc.input) } else { -- cgit v1.2.3-54-g00ecf From fe1230ef7003e89ad9c92fd11e21494ecc64de85 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 19:21:02 +0200 Subject: Remove pkg/auth.Header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is no longer used. Split the existing tests into MakeXRegistryConfigHeader and MakeXRegistryAuthHeader variants. For now we don't modify the implementations at all, to make review simpler; cleanups will follow. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 13 ------ pkg/auth/auth_test.go | 123 ++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 100 insertions(+), 36 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 84b2f8ce6..b68109429 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -3,7 +3,6 @@ package auth import ( "encoding/base64" "encoding/json" - "fmt" "io/ioutil" "net/http" "os" @@ -143,18 +142,6 @@ func getAuthCredentials(headers []string) (*types.DockerAuthConfig, map[string]t return &authConfig, nil, nil } -// Header builds the requested Authentication Header -func Header(sys *types.SystemContext, headerName HeaderAuthName, authfile, username, password string) (map[string]string, error) { - switch headerName { - case XRegistryAuthHeader: - return MakeXRegistryAuthHeader(sys, authfile, username, password) - case XRegistryConfigHeader: - return MakeXRegistryConfigHeader(sys, authfile, username, password) - default: - return nil, fmt.Errorf("unsupported authentication header: %q", headerName) - } -} - // MakeXRegistryConfigHeader returns a map with the XRegistryConfigHeader set which can // conveniently be used in the http stack. func MakeXRegistryConfigHeader(sys *types.SystemContext, authfile, username, password string) (map[string]string, error) { diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index 6acf1f8fb..e0d2f1ac6 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -3,7 +3,6 @@ package auth import ( "encoding/base64" "encoding/json" - "fmt" "io/ioutil" "net/http" "os" @@ -30,10 +29,9 @@ var largeAuthFileValues = map[string]types.DockerAuthConfig{ "quay.io": {Username: "quay", Password: "top"}, } -// Test that GetCredentials() correctly parses what Header() produces -func TestHeaderGetCredentialsRoundtrip(t *testing.T) { +// Test that GetCredentials() correctly parses what MakeXRegistryConfigHeader() produces +func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { for _, tc := range []struct { - headerName HeaderAuthName name string fileContents string username, password string @@ -41,7 +39,6 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues map[string]types.DockerAuthConfig }{ { - headerName: XRegistryConfigHeader, name: "no data", fileContents: "", username: "", @@ -50,7 +47,6 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: nil, }, { - headerName: XRegistryConfigHeader, name: "file data", fileContents: largeAuthFile, username: "", @@ -59,7 +55,6 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, { - headerName: XRegistryConfigHeader, name: "file data + override", fileContents: largeAuthFile, username: "override-user", @@ -67,8 +62,53 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedOverride: &types.DockerAuthConfig{Username: "override-user", Password: "override-pass"}, expectedFileValues: largeAuthFileValues, }, + } { + name := tc.name + inputAuthFile := "" + if tc.fileContents != "" { + f, err := ioutil.TempFile("", "auth.json") + require.NoError(t, err, name) + defer os.Remove(f.Name()) + inputAuthFile = f.Name() + err = ioutil.WriteFile(inputAuthFile, []byte(tc.fileContents), 0700) + require.NoError(t, err, name) + } + + headers, err := MakeXRegistryConfigHeader(nil, inputAuthFile, tc.username, tc.password) + require.NoError(t, err) + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err, name) + for k, v := range headers { + req.Header.Set(k, v) + } + + override, resPath, err := GetCredentials(req) + require.NoError(t, err, name) + defer RemoveAuthfile(resPath) + if tc.expectedOverride == nil { + assert.Nil(t, override, name) + } else { + require.NotNil(t, override, name) + assert.Equal(t, *tc.expectedOverride, *override, name) + } + for key, expectedAuth := range tc.expectedFileValues { + auth, err := config.GetCredentials(&types.SystemContext{AuthFilePath: resPath}, key) + require.NoError(t, err, name) + assert.Equal(t, expectedAuth, auth, "%s, key %s", name, key) + } + } +} + +// Test that GetCredentials() correctly parses what MakeXRegistryAuthHeader() produces +func TestMakeXRegistryAuthHeaderGetCredentialsRoundtrip(t *testing.T) { + for _, tc := range []struct { + name string + fileContents string + username, password string + expectedOverride *types.DockerAuthConfig + expectedFileValues map[string]types.DockerAuthConfig + }{ { - headerName: XRegistryAuthHeader, name: "override", fileContents: "", username: "override-user", @@ -77,7 +117,6 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: nil, }, { - headerName: XRegistryAuthHeader, name: "file data", fileContents: largeAuthFile, username: "", @@ -85,7 +124,7 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, } { - name := fmt.Sprintf("%s: %s", tc.headerName, tc.name) + name := tc.name inputAuthFile := "" if tc.fileContents != "" { f, err := ioutil.TempFile("", "auth.json") @@ -96,7 +135,7 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { require.NoError(t, err, name) } - headers, err := Header(nil, tc.headerName, inputAuthFile, tc.username, tc.password) + headers, err := MakeXRegistryAuthHeader(nil, inputAuthFile, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err, name) @@ -121,9 +160,8 @@ func TestHeaderGetCredentialsRoundtrip(t *testing.T) { } } -func TestHeader(t *testing.T) { +func TestMakeXRegistryConfigHeader(t *testing.T) { for _, tc := range []struct { - headerName HeaderAuthName name string fileContents string username, password string @@ -131,7 +169,6 @@ func TestHeader(t *testing.T) { expectedContents string }{ { - headerName: XRegistryConfigHeader, name: "no data", fileContents: "", username: "", @@ -139,7 +176,6 @@ func TestHeader(t *testing.T) { expectedContents: "", }, { - headerName: XRegistryConfigHeader, name: "invalid JSON", fileContents: "@invalid JSON", username: "", @@ -147,7 +183,6 @@ func TestHeader(t *testing.T) { shouldErr: true, }, { - headerName: XRegistryConfigHeader, name: "file data", fileContents: largeAuthFile, username: "", @@ -160,7 +195,6 @@ func TestHeader(t *testing.T) { }`, }, { - headerName: XRegistryConfigHeader, name: "file data + override", fileContents: largeAuthFile, username: "override-user", @@ -173,8 +207,53 @@ func TestHeader(t *testing.T) { "": {"username": "override-user", "password": "override-pass"} }`, }, + } { + name := tc.name + authFile := "" + if tc.fileContents != "" { + f, err := ioutil.TempFile("", "auth.json") + require.NoError(t, err, name) + defer os.Remove(f.Name()) + authFile = f.Name() + err = ioutil.WriteFile(authFile, []byte(tc.fileContents), 0700) + require.NoError(t, err, name) + } + + res, err := MakeXRegistryConfigHeader(nil, authFile, tc.username, tc.password) + if tc.shouldErr { + assert.Error(t, err, name) + } else { + require.NoError(t, err, name) + if tc.expectedContents == "" { + assert.Empty(t, res, name) + } else { + require.Len(t, res, 1, name) + header, ok := res[XRegistryConfigHeader.String()] + require.True(t, ok, name) + decodedHeader, err := base64.URLEncoding.DecodeString(header) + require.NoError(t, err, name) + // Don't test for a specific JSON representation, just for the expected contents. + expected := map[string]interface{}{} + actual := map[string]interface{}{} + err = json.Unmarshal([]byte(tc.expectedContents), &expected) + require.NoError(t, err, name) + err = json.Unmarshal(decodedHeader, &actual) + require.NoError(t, err, name) + assert.Equal(t, expected, actual, name) + } + } + } +} + +func TestMakeXRegistryAuthHeader(t *testing.T) { + for _, tc := range []struct { + name string + fileContents string + username, password string + shouldErr bool + expectedContents string + }{ { - headerName: XRegistryAuthHeader, name: "override", fileContents: "", username: "override-user", @@ -182,7 +261,6 @@ func TestHeader(t *testing.T) { expectedContents: `{"username": "override-user", "password": "override-pass"}`, }, { - headerName: XRegistryAuthHeader, name: "invalid JSON", fileContents: "@invalid JSON", username: "", @@ -190,7 +268,6 @@ func TestHeader(t *testing.T) { shouldErr: true, }, { - headerName: XRegistryAuthHeader, name: "file data", fileContents: largeAuthFile, username: "", @@ -203,7 +280,7 @@ func TestHeader(t *testing.T) { }`, }, } { - name := fmt.Sprintf("%s: %s", tc.headerName, tc.name) + name := tc.name authFile := "" if tc.fileContents != "" { f, err := ioutil.TempFile("", "auth.json") @@ -214,7 +291,7 @@ func TestHeader(t *testing.T) { require.NoError(t, err, name) } - res, err := Header(nil, tc.headerName, authFile, tc.username, tc.password) + res, err := MakeXRegistryAuthHeader(nil, authFile, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, name) } else { @@ -223,7 +300,7 @@ func TestHeader(t *testing.T) { assert.Empty(t, res, name) } else { require.Len(t, res, 1, name) - header, ok := res[tc.headerName.String()] + header, ok := res[XRegistryAuthHeader.String()] require.True(t, ok, name) decodedHeader, err := base64.URLEncoding.DecodeString(header) require.NoError(t, err, name) -- cgit v1.2.3-54-g00ecf From 0e29b89753cc252076d171acdcb5b36aa386ba3e Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 20:44:15 +0200 Subject: Consolidate creation of SystemContext with auth.json into a helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change (test) behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 64 +++++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 40 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index e0d2f1ac6..cbba600a9 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -29,6 +29,22 @@ var largeAuthFileValues = map[string]types.DockerAuthConfig{ "quay.io": {Username: "quay", Password: "top"}, } +// tempAuthFilePath returns a non-empty path pointing +// to a temporary file with fileContents, or "" if fileContents is empty; and a cleanup +// function the caller must arrange to call. +func tempAuthFilePath(t *testing.T, fileContents string) (string, func()) { + if fileContents == "" { + return "", func() {} + } + + f, err := ioutil.TempFile("", "auth.json") + require.NoError(t, err) + path := f.Name() + err = ioutil.WriteFile(path, []byte(fileContents), 0700) + require.NoError(t, err) + return path, func() { os.Remove(path) } +} + // Test that GetCredentials() correctly parses what MakeXRegistryConfigHeader() produces func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { for _, tc := range []struct { @@ -64,16 +80,8 @@ func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { }, } { name := tc.name - inputAuthFile := "" - if tc.fileContents != "" { - f, err := ioutil.TempFile("", "auth.json") - require.NoError(t, err, name) - defer os.Remove(f.Name()) - inputAuthFile = f.Name() - err = ioutil.WriteFile(inputAuthFile, []byte(tc.fileContents), 0700) - require.NoError(t, err, name) - } - + inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) + defer cleanup() headers, err := MakeXRegistryConfigHeader(nil, inputAuthFile, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) @@ -125,16 +133,8 @@ func TestMakeXRegistryAuthHeaderGetCredentialsRoundtrip(t *testing.T) { }, } { name := tc.name - inputAuthFile := "" - if tc.fileContents != "" { - f, err := ioutil.TempFile("", "auth.json") - require.NoError(t, err, name) - defer os.Remove(f.Name()) - inputAuthFile = f.Name() - err = ioutil.WriteFile(inputAuthFile, []byte(tc.fileContents), 0700) - require.NoError(t, err, name) - } - + inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) + defer cleanup() headers, err := MakeXRegistryAuthHeader(nil, inputAuthFile, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) @@ -209,16 +209,8 @@ func TestMakeXRegistryConfigHeader(t *testing.T) { }, } { name := tc.name - authFile := "" - if tc.fileContents != "" { - f, err := ioutil.TempFile("", "auth.json") - require.NoError(t, err, name) - defer os.Remove(f.Name()) - authFile = f.Name() - err = ioutil.WriteFile(authFile, []byte(tc.fileContents), 0700) - require.NoError(t, err, name) - } - + authFile, cleanup := tempAuthFilePath(t, tc.fileContents) + defer cleanup() res, err := MakeXRegistryConfigHeader(nil, authFile, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, name) @@ -281,16 +273,8 @@ func TestMakeXRegistryAuthHeader(t *testing.T) { }, } { name := tc.name - authFile := "" - if tc.fileContents != "" { - f, err := ioutil.TempFile("", "auth.json") - require.NoError(t, err, name) - defer os.Remove(f.Name()) - authFile = f.Name() - err = ioutil.WriteFile(authFile, []byte(tc.fileContents), 0700) - require.NoError(t, err, name) - } - + authFile, cleanup := tempAuthFilePath(t, tc.fileContents) + defer cleanup() res, err := MakeXRegistryAuthHeader(nil, authFile, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, name) -- cgit v1.2.3-54-g00ecf From 935dcbb0088a667406dd00562ea8fc1fbc00f3ef Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 19:24:56 +0200 Subject: Remove no-longer-useful name variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit which used to contain more context, but now are just a pointless copy. Should not change (test) behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth_test.go | 68 ++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 36 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index cbba600a9..ec5c1c9e7 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -79,30 +79,29 @@ func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, } { - name := tc.name inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() headers, err := MakeXRegistryConfigHeader(nil, inputAuthFile, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) for k, v := range headers { req.Header.Set(k, v) } override, resPath, err := GetCredentials(req) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) defer RemoveAuthfile(resPath) if tc.expectedOverride == nil { - assert.Nil(t, override, name) + assert.Nil(t, override, tc.name) } else { - require.NotNil(t, override, name) - assert.Equal(t, *tc.expectedOverride, *override, name) + require.NotNil(t, override, tc.name) + assert.Equal(t, *tc.expectedOverride, *override, tc.name) } for key, expectedAuth := range tc.expectedFileValues { auth, err := config.GetCredentials(&types.SystemContext{AuthFilePath: resPath}, key) - require.NoError(t, err, name) - assert.Equal(t, expectedAuth, auth, "%s, key %s", name, key) + require.NoError(t, err, tc.name) + assert.Equal(t, expectedAuth, auth, "%s, key %s", tc.name, key) } } } @@ -132,30 +131,29 @@ func TestMakeXRegistryAuthHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, } { - name := tc.name inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() headers, err := MakeXRegistryAuthHeader(nil, inputAuthFile, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) for k, v := range headers { req.Header.Set(k, v) } override, resPath, err := GetCredentials(req) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) defer RemoveAuthfile(resPath) if tc.expectedOverride == nil { - assert.Nil(t, override, name) + assert.Nil(t, override, tc.name) } else { - require.NotNil(t, override, name) - assert.Equal(t, *tc.expectedOverride, *override, name) + require.NotNil(t, override, tc.name) + assert.Equal(t, *tc.expectedOverride, *override, tc.name) } for key, expectedAuth := range tc.expectedFileValues { auth, err := config.GetCredentials(&types.SystemContext{AuthFilePath: resPath}, key) - require.NoError(t, err, name) - assert.Equal(t, expectedAuth, auth, "%s, key %s", name, key) + require.NoError(t, err, tc.name) + assert.Equal(t, expectedAuth, auth, "%s, key %s", tc.name, key) } } } @@ -208,30 +206,29 @@ func TestMakeXRegistryConfigHeader(t *testing.T) { }`, }, } { - name := tc.name authFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() res, err := MakeXRegistryConfigHeader(nil, authFile, tc.username, tc.password) if tc.shouldErr { - assert.Error(t, err, name) + assert.Error(t, err, tc.name) } else { - require.NoError(t, err, name) + require.NoError(t, err, tc.name) if tc.expectedContents == "" { - assert.Empty(t, res, name) + assert.Empty(t, res, tc.name) } else { - require.Len(t, res, 1, name) + require.Len(t, res, 1, tc.name) header, ok := res[XRegistryConfigHeader.String()] - require.True(t, ok, name) + require.True(t, ok, tc.name) decodedHeader, err := base64.URLEncoding.DecodeString(header) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) // Don't test for a specific JSON representation, just for the expected contents. expected := map[string]interface{}{} actual := map[string]interface{}{} err = json.Unmarshal([]byte(tc.expectedContents), &expected) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) err = json.Unmarshal(decodedHeader, &actual) - require.NoError(t, err, name) - assert.Equal(t, expected, actual, name) + require.NoError(t, err, tc.name) + assert.Equal(t, expected, actual, tc.name) } } } @@ -272,30 +269,29 @@ func TestMakeXRegistryAuthHeader(t *testing.T) { }`, }, } { - name := tc.name authFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() res, err := MakeXRegistryAuthHeader(nil, authFile, tc.username, tc.password) if tc.shouldErr { - assert.Error(t, err, name) + assert.Error(t, err, tc.name) } else { - require.NoError(t, err, name) + require.NoError(t, err, tc.name) if tc.expectedContents == "" { - assert.Empty(t, res, name) + assert.Empty(t, res, tc.name) } else { - require.Len(t, res, 1, name) + require.Len(t, res, 1, tc.name) header, ok := res[XRegistryAuthHeader.String()] - require.True(t, ok, name) + require.True(t, ok, tc.name) decodedHeader, err := base64.URLEncoding.DecodeString(header) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) // Don't test for a specific JSON representation, just for the expected contents. expected := map[string]interface{}{} actual := map[string]interface{}{} err = json.Unmarshal([]byte(tc.expectedContents), &expected) - require.NoError(t, err, name) + require.NoError(t, err, tc.name) err = json.Unmarshal(decodedHeader, &actual) - require.NoError(t, err, name) - assert.Equal(t, expected, actual, name) + require.NoError(t, err, tc.name) + assert.Equal(t, expected, actual, tc.name) } } } -- cgit v1.2.3-54-g00ecf From f9be3262740a2961f5c1a4db24265234a3fc6a5f Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 21:27:25 +0200 Subject: Remove the authfile parameter of MakeXRegistryConfigHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having a parameter that modifies the provides types.SystemContext seems rather unexpected and risky to have around - and the only user of that is actually a no-op; so, remove that option and simplify. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 5 +---- pkg/auth/auth_test.go | 4 ++-- pkg/bindings/images/build.go | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index b68109429..006572b09 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -144,13 +144,10 @@ func getAuthCredentials(headers []string) (*types.DockerAuthConfig, map[string]t // MakeXRegistryConfigHeader returns a map with the XRegistryConfigHeader set which can // conveniently be used in the http stack. -func MakeXRegistryConfigHeader(sys *types.SystemContext, authfile, username, password string) (map[string]string, error) { +func MakeXRegistryConfigHeader(sys *types.SystemContext, username, password string) (map[string]string, error) { if sys == nil { sys = &types.SystemContext{} } - if authfile != "" { - sys.AuthFilePath = authfile - } authConfigs, err := imageAuth.GetAllCredentials(sys) if err != nil { return nil, err diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index ec5c1c9e7..a727a9d50 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -81,7 +81,7 @@ func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { } { inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() - headers, err := MakeXRegistryConfigHeader(nil, inputAuthFile, tc.username, tc.password) + headers, err := MakeXRegistryConfigHeader(&types.SystemContext{AuthFilePath: inputAuthFile}, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err, tc.name) @@ -208,7 +208,7 @@ func TestMakeXRegistryConfigHeader(t *testing.T) { } { authFile, cleanup := tempAuthFilePath(t, tc.fileContents) defer cleanup() - res, err := MakeXRegistryConfigHeader(nil, authFile, tc.username, tc.password) + res, err := MakeXRegistryConfigHeader(&types.SystemContext{AuthFilePath: authFile}, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, tc.name) } else { diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index f643b3c89..54c831c36 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -294,12 +294,12 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO err error ) if options.SystemContext == nil { - headers, err = auth.MakeXRegistryConfigHeader(options.SystemContext, "", "", "") + headers, err = auth.MakeXRegistryConfigHeader(options.SystemContext, "", "") } else { if options.SystemContext.DockerAuthConfig != nil { headers, err = auth.MakeXRegistryAuthHeader(options.SystemContext, options.SystemContext.AuthFilePath, options.SystemContext.DockerAuthConfig.Username, options.SystemContext.DockerAuthConfig.Password) } else { - headers, err = auth.MakeXRegistryConfigHeader(options.SystemContext, options.SystemContext.AuthFilePath, "", "") + headers, err = auth.MakeXRegistryConfigHeader(options.SystemContext, "", "") } } if err != nil { -- cgit v1.2.3-54-g00ecf From 3cfefa1248feb9de8041b9fc67987b508d4c3fcb Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 21:31:22 +0200 Subject: Remove the authfile parameter of MakeXRegistryAuthHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having a parameter that modifies the provides types.SystemContext seems rather unexpected and risky to have around - and the only user of that is actually a no-op, others only provide a nil SystemContext; so, remove that option and simplify (well, somewhat; many callers now have extra &types.SystemContext{AuthFilePath} boilerplate; at least that's consistent with that code carrying a TODO to create a larger-scope SystemContext). Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 5 +---- pkg/auth/auth_test.go | 28 ++++++++++++++-------------- pkg/bindings/images/build.go | 2 +- pkg/bindings/images/images.go | 5 +++-- pkg/bindings/images/pull.go | 3 ++- pkg/bindings/play/play.go | 3 ++- 6 files changed, 23 insertions(+), 23 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 006572b09..2124b5302 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -172,7 +172,7 @@ func MakeXRegistryConfigHeader(sys *types.SystemContext, username, password stri // MakeXRegistryAuthHeader returns a map with the XRegistryAuthHeader set which can // conveniently be used in the http stack. -func MakeXRegistryAuthHeader(sys *types.SystemContext, authfile, username, password string) (map[string]string, error) { +func MakeXRegistryAuthHeader(sys *types.SystemContext, username, password string) (map[string]string, error) { if username != "" { content, err := encodeSingleAuthConfig(types.DockerAuthConfig{Username: username, Password: password}) if err != nil { @@ -184,9 +184,6 @@ func MakeXRegistryAuthHeader(sys *types.SystemContext, authfile, username, passw if sys == nil { sys = &types.SystemContext{} } - if authfile != "" { - sys.AuthFilePath = authfile - } authConfigs, err := imageAuth.GetAllCredentials(sys) if err != nil { return nil, err diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index a727a9d50..bce488a91 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -29,12 +29,12 @@ var largeAuthFileValues = map[string]types.DockerAuthConfig{ "quay.io": {Username: "quay", Password: "top"}, } -// tempAuthFilePath returns a non-empty path pointing -// to a temporary file with fileContents, or "" if fileContents is empty; and a cleanup -// function the caller must arrange to call. -func tempAuthFilePath(t *testing.T, fileContents string) (string, func()) { +// systemContextForAuthFile returns a types.SystemContext with AuthFilePath pointing +// to a temporary file with fileContents, or nil if fileContents is empty; and a cleanup +// function the calle rmust arrange to call. +func systemContextForAuthFile(t *testing.T, fileContents string) (*types.SystemContext, func()) { if fileContents == "" { - return "", func() {} + return nil, func() {} } f, err := ioutil.TempFile("", "auth.json") @@ -42,7 +42,7 @@ func tempAuthFilePath(t *testing.T, fileContents string) (string, func()) { path := f.Name() err = ioutil.WriteFile(path, []byte(fileContents), 0700) require.NoError(t, err) - return path, func() { os.Remove(path) } + return &types.SystemContext{AuthFilePath: path}, func() { os.Remove(path) } } // Test that GetCredentials() correctly parses what MakeXRegistryConfigHeader() produces @@ -79,9 +79,9 @@ func TestMakeXRegistryConfigHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, } { - inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) + sys, cleanup := systemContextForAuthFile(t, tc.fileContents) defer cleanup() - headers, err := MakeXRegistryConfigHeader(&types.SystemContext{AuthFilePath: inputAuthFile}, tc.username, tc.password) + headers, err := MakeXRegistryConfigHeader(sys, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err, tc.name) @@ -131,9 +131,9 @@ func TestMakeXRegistryAuthHeaderGetCredentialsRoundtrip(t *testing.T) { expectedFileValues: largeAuthFileValues, }, } { - inputAuthFile, cleanup := tempAuthFilePath(t, tc.fileContents) + sys, cleanup := systemContextForAuthFile(t, tc.fileContents) defer cleanup() - headers, err := MakeXRegistryAuthHeader(nil, inputAuthFile, tc.username, tc.password) + headers, err := MakeXRegistryAuthHeader(sys, tc.username, tc.password) require.NoError(t, err) req, err := http.NewRequest(http.MethodPost, "/", nil) require.NoError(t, err, tc.name) @@ -206,9 +206,9 @@ func TestMakeXRegistryConfigHeader(t *testing.T) { }`, }, } { - authFile, cleanup := tempAuthFilePath(t, tc.fileContents) + sys, cleanup := systemContextForAuthFile(t, tc.fileContents) defer cleanup() - res, err := MakeXRegistryConfigHeader(&types.SystemContext{AuthFilePath: authFile}, tc.username, tc.password) + res, err := MakeXRegistryConfigHeader(sys, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, tc.name) } else { @@ -269,9 +269,9 @@ func TestMakeXRegistryAuthHeader(t *testing.T) { }`, }, } { - authFile, cleanup := tempAuthFilePath(t, tc.fileContents) + sys, cleanup := systemContextForAuthFile(t, tc.fileContents) defer cleanup() - res, err := MakeXRegistryAuthHeader(nil, authFile, tc.username, tc.password) + res, err := MakeXRegistryAuthHeader(sys, tc.username, tc.password) if tc.shouldErr { assert.Error(t, err, tc.name) } else { diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index fece5e9d0..7bca43132 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -294,7 +294,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO err error ) if options.SystemContext != nil && options.SystemContext.DockerAuthConfig != nil { - headers, err = auth.MakeXRegistryAuthHeader(options.SystemContext, options.SystemContext.AuthFilePath, options.SystemContext.DockerAuthConfig.Username, options.SystemContext.DockerAuthConfig.Password) + headers, err = auth.MakeXRegistryAuthHeader(options.SystemContext, options.SystemContext.DockerAuthConfig.Username, options.SystemContext.DockerAuthConfig.Password) } else { headers, err = auth.MakeXRegistryConfigHeader(options.SystemContext, "", "") } diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go index 74603015c..152ff0cde 100644 --- a/pkg/bindings/images/images.go +++ b/pkg/bindings/images/images.go @@ -8,6 +8,7 @@ import ( "net/url" "strconv" + imageTypes "github.com/containers/image/v5/types" "github.com/containers/podman/v3/pkg/api/handlers/types" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" @@ -280,7 +281,7 @@ func Push(ctx context.Context, source string, destination string, options *PushO return err } // TODO: have a global system context we can pass around (1st argument) - header, err := auth.MakeXRegistryAuthHeader(nil, options.GetAuthfile(), options.GetUsername(), options.GetPassword()) + header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, options.GetUsername(), options.GetPassword()) if err != nil { return err } @@ -329,7 +330,7 @@ func Search(ctx context.Context, term string, options *SearchOptions) ([]entitie } // TODO: have a global system context we can pass around (1st argument) - header, err := auth.MakeXRegistryAuthHeader(nil, options.GetAuthfile(), "", "") + header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, "", "") if err != nil { return nil, err } diff --git a/pkg/bindings/images/pull.go b/pkg/bindings/images/pull.go index c6f20e3e1..ac583973f 100644 --- a/pkg/bindings/images/pull.go +++ b/pkg/bindings/images/pull.go @@ -10,6 +10,7 @@ import ( "os" "strconv" + "github.com/containers/image/v5/types" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" @@ -42,7 +43,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string, } // TODO: have a global system context we can pass around (1st argument) - header, err := auth.MakeXRegistryAuthHeader(nil, options.GetAuthfile(), options.GetUsername(), options.GetPassword()) + header, err := auth.MakeXRegistryAuthHeader(&types.SystemContext{AuthFilePath: options.GetAuthfile()}, options.GetUsername(), options.GetPassword()) if err != nil { return nil, err } diff --git a/pkg/bindings/play/play.go b/pkg/bindings/play/play.go index 64a2ae6ae..111a25cac 100644 --- a/pkg/bindings/play/play.go +++ b/pkg/bindings/play/play.go @@ -6,6 +6,7 @@ import ( "os" "strconv" + "github.com/containers/image/v5/types" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" @@ -40,7 +41,7 @@ func Kube(ctx context.Context, path string, options *KubeOptions) (*entities.Pla } // TODO: have a global system context we can pass around (1st argument) - header, err := auth.MakeXRegistryAuthHeader(nil, options.GetAuthfile(), options.GetUsername(), options.GetPassword()) + header, err := auth.MakeXRegistryAuthHeader(&types.SystemContext{AuthFilePath: options.GetAuthfile()}, options.GetUsername(), options.GetPassword()) if err != nil { return nil, err } -- cgit v1.2.3-54-g00ecf From 5bbcfaf4aa4f276423d9fae31aee5945b7f5a9b8 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Thu, 21 Oct 2021 20:52:38 +0200 Subject: Make XRegistryAuthHeader and XRegistryConfigHeader private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... now that they have no public users. Also remove the HeaderAuthName type, we don't need the type-safety so much for private constants, and using plain strings results in less visual noise. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/auth/auth.go | 52 ++++++++++++++++++++++++--------------------------- pkg/auth/auth_test.go | 4 ++-- 2 files changed, 26 insertions(+), 30 deletions(-) (limited to 'pkg/auth/auth_test.go') diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 2124b5302..f423c011d 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -15,37 +15,33 @@ import ( "github.com/sirupsen/logrus" ) -type HeaderAuthName string - -func (h HeaderAuthName) String() string { return string(h) } - -// XRegistryAuthHeader is the key to the encoded registry authentication configuration in an http-request header. +// xRegistryAuthHeader is the key to the encoded registry authentication configuration in an http-request header. // This header supports one registry per header occurrence. To support N registries provide N headers, one per registry. // As of Docker API 1.40 and Libpod API 1.0.0, this header is supported by all endpoints. -const XRegistryAuthHeader HeaderAuthName = "X-Registry-Auth" +const xRegistryAuthHeader = "X-Registry-Auth" -// XRegistryConfigHeader is the key to the encoded registry authentication configuration in an http-request header. +// xRegistryConfigHeader is the key to the encoded registry authentication configuration in an http-request header. // This header supports N registries in one header via a Base64 encoded, JSON map. // As of Docker API 1.40 and Libpod API 2.0.0, this header is supported by build endpoints. -const XRegistryConfigHeader HeaderAuthName = "X-Registry-Config" +const xRegistryConfigHeader = "X-Registry-Config" // GetCredentials queries the http.Request for X-Registry-.* headers and extracts // the necessary authentication information for libpod operations, possibly // creating a config file. If that is the case, the caller must call RemoveAuthFile. func GetCredentials(r *http.Request) (*types.DockerAuthConfig, string, error) { - nonemptyHeaderValue := func(key HeaderAuthName) ([]string, bool) { - hdr := r.Header.Values(key.String()) + nonemptyHeaderValue := func(key string) ([]string, bool) { + hdr := r.Header.Values(key) return hdr, len(hdr) > 0 } var override *types.DockerAuthConfig var fileContents map[string]types.DockerAuthConfig - var headerName HeaderAuthName + var headerName string var err error - if hdr, ok := nonemptyHeaderValue(XRegistryConfigHeader); ok { - headerName = XRegistryConfigHeader + if hdr, ok := nonemptyHeaderValue(xRegistryConfigHeader); ok { + headerName = xRegistryConfigHeader override, fileContents, err = getConfigCredentials(r, hdr) - } else if hdr, ok := nonemptyHeaderValue(XRegistryAuthHeader); ok { - headerName = XRegistryAuthHeader + } else if hdr, ok := nonemptyHeaderValue(xRegistryAuthHeader); ok { + headerName = xRegistryAuthHeader override, fileContents, err = getAuthCredentials(hdr) } else { return nil, "", nil @@ -67,7 +63,7 @@ func GetCredentials(r *http.Request) (*types.DockerAuthConfig, string, error) { } // getConfigCredentials extracts one or more docker.AuthConfig from a request and its -// XRegistryConfigHeader value. An empty key will be used as default while a named registry will be +// xRegistryConfigHeader value. An empty key will be used as default while a named registry will be // returned as types.DockerAuthConfig func getConfigCredentials(r *http.Request, headers []string) (*types.DockerAuthConfig, map[string]types.DockerAuthConfig, error) { var auth *types.DockerAuthConfig @@ -76,13 +72,13 @@ func getConfigCredentials(r *http.Request, headers []string) (*types.DockerAuthC for _, h := range headers { param, err := base64.URLEncoding.DecodeString(h) if err != nil { - return nil, nil, errors.Wrapf(err, "failed to decode %q", XRegistryConfigHeader) + return nil, nil, errors.Wrapf(err, "failed to decode %q", xRegistryConfigHeader) } ac := make(map[string]dockerAPITypes.AuthConfig) err = json.Unmarshal(param, &ac) if err != nil { - return nil, nil, errors.Wrapf(err, "failed to unmarshal %q", XRegistryConfigHeader) + return nil, nil, errors.Wrapf(err, "failed to unmarshal %q", xRegistryConfigHeader) } for k, v := range ac { @@ -112,16 +108,16 @@ func getConfigCredentials(r *http.Request, headers []string) (*types.DockerAuthC if auth == nil { logrus.Debugf("%q header found in request, but \"registry=%v\" query parameter not provided", - XRegistryConfigHeader, registries) + xRegistryConfigHeader, registries) } else { - logrus.Debugf("%q header found in request for username %q", XRegistryConfigHeader, auth.Username) + logrus.Debugf("%q header found in request for username %q", xRegistryConfigHeader, auth.Username) } } return auth, configs, nil } -// getAuthCredentials extracts one or more DockerAuthConfigs from an XRegistryAuthHeader +// getAuthCredentials extracts one or more DockerAuthConfigs from an xRegistryAuthHeader // value. The header could specify a single-auth config in which case the // first return value is set. In case of a multi-auth header, the contents are // returned in the second return value. @@ -142,7 +138,7 @@ func getAuthCredentials(headers []string) (*types.DockerAuthConfig, map[string]t return &authConfig, nil, nil } -// MakeXRegistryConfigHeader returns a map with the XRegistryConfigHeader set which can +// MakeXRegistryConfigHeader returns a map with the "X-Registry-Config" header set, which can // conveniently be used in the http stack. func MakeXRegistryConfigHeader(sys *types.SystemContext, username, password string) (map[string]string, error) { if sys == nil { @@ -167,10 +163,10 @@ func MakeXRegistryConfigHeader(sys *types.SystemContext, username, password stri if err != nil { return nil, err } - return map[string]string{XRegistryConfigHeader.String(): content}, nil + return map[string]string{xRegistryConfigHeader: content}, nil } -// MakeXRegistryAuthHeader returns a map with the XRegistryAuthHeader set which can +// MakeXRegistryAuthHeader returns a map with the "X-Registry-Auth" header set, which can // conveniently be used in the http stack. func MakeXRegistryAuthHeader(sys *types.SystemContext, username, password string) (map[string]string, error) { if username != "" { @@ -178,7 +174,7 @@ func MakeXRegistryAuthHeader(sys *types.SystemContext, username, password string if err != nil { return nil, err } - return map[string]string{XRegistryAuthHeader.String(): content}, nil + return map[string]string{xRegistryAuthHeader: content}, nil } if sys == nil { @@ -192,7 +188,7 @@ func MakeXRegistryAuthHeader(sys *types.SystemContext, username, password string if err != nil { return nil, err } - return map[string]string{XRegistryAuthHeader.String(): content}, nil + return map[string]string{xRegistryAuthHeader: content}, nil } // RemoveAuthfile is a convenience function that is meant to be called in a @@ -309,7 +305,7 @@ func imageAuthToDockerAuth(authConfig types.DockerAuthConfig) dockerAPITypes.Aut } } -// parseSingleAuthHeader extracts a DockerAuthConfig from an XRegistryAuthHeader value. +// parseSingleAuthHeader extracts a DockerAuthConfig from an xRegistryAuthHeader value. // The header content is a single DockerAuthConfig. func parseSingleAuthHeader(authHeader string) (types.DockerAuthConfig, error) { // Accept "null" and handle it as empty value for compatibility reason with Docker. @@ -326,7 +322,7 @@ func parseSingleAuthHeader(authHeader string) (types.DockerAuthConfig, error) { return dockerAuthToImageAuth(authConfig), nil } -// parseMultiAuthHeader extracts a DockerAuthConfig from an XRegistryAuthHeader value. +// parseMultiAuthHeader extracts a DockerAuthConfig from an xRegistryAuthHeader value. // The header content is a map[string]DockerAuthConfigs. func parseMultiAuthHeader(authHeader string) (map[string]types.DockerAuthConfig, error) { // Accept "null" and handle it as empty value for compatibility reason with Docker. diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go index bce488a91..f7e6e4ef6 100644 --- a/pkg/auth/auth_test.go +++ b/pkg/auth/auth_test.go @@ -217,7 +217,7 @@ func TestMakeXRegistryConfigHeader(t *testing.T) { assert.Empty(t, res, tc.name) } else { require.Len(t, res, 1, tc.name) - header, ok := res[XRegistryConfigHeader.String()] + header, ok := res[xRegistryConfigHeader] require.True(t, ok, tc.name) decodedHeader, err := base64.URLEncoding.DecodeString(header) require.NoError(t, err, tc.name) @@ -280,7 +280,7 @@ func TestMakeXRegistryAuthHeader(t *testing.T) { assert.Empty(t, res, tc.name) } else { require.Len(t, res, 1, tc.name) - header, ok := res[XRegistryAuthHeader.String()] + header, ok := res[xRegistryAuthHeader] require.True(t, ok, tc.name) decodedHeader, err := base64.URLEncoding.DecodeString(header) require.NoError(t, err, tc.name) -- cgit v1.2.3-54-g00ecf