From bdb15f4f2d45f7e4ebcf60af31d5962113c082ef Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 24 Sep 2024 19:41:01 +0000 Subject: [PATCH] Fix tests that maybe never worked These started failing after changes to the https backend. After inspection, I'm surprised they'd been passing at all, and am not convinced they were actually testing what we wanted them to. At the very least, they were grabbing config from my system installation of Pelican (NAUGHTY), and this correctly isolates them. --- client/fed_test.go | 19 +++----- client/resources/test-https-origin.yml | 3 +- cmd/plugin_test.go | 19 ++++---- cmd/resources/test-https-origin.yml | 3 +- xrootd/fed_test.go | 67 +++++--------------------- xrootd/resources/test-https-origin.yml | 4 +- 6 files changed, 35 insertions(+), 80 deletions(-) diff --git a/client/fed_test.go b/client/fed_test.go index 93e2176f4..a199f0855 100644 --- a/client/fed_test.go +++ b/client/fed_test.go @@ -45,7 +45,6 @@ import ( "github.com/pelicanplatform/pelican/fed_test_utils" "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_utils" - "github.com/pelicanplatform/pelican/test_utils" "github.com/pelicanplatform/pelican/token" "github.com/pelicanplatform/pelican/token_scopes" "github.com/pelicanplatform/pelican/utils" @@ -867,37 +866,35 @@ func TestObjectList(t *testing.T) { // This tests object ls but for an origin that supports listings but with an object store that does not support PROPFIND. // We should get a 405 returned. This is a separate test since we need a completely different origin func TestObjectList405Error(t *testing.T) { - test_utils.InitClient(t, nil) server_utils.ResetOriginExports() defer server_utils.ResetOriginExports() - err := config.InitClient() - require.NoError(t, err) + var storageName string // Set up our http backend so that we can return a 405 on a PROPFIND body := "Hello, World!" srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" { + if r.Method == "HEAD" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusOK) return - } else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" { + } else if r.Method == "GET" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusPartialContent) _, err := w.Write([]byte(body)) require.NoError(t, err) return - } else if r.Method == "PROPFIND" && r.URL.Path == "/test2/hello_world" { + } else if r.Method == "PROPFIND" && r.URL.Path == storageName { w.WriteHeader(http.StatusMethodNotAllowed) } })) defer srv.Close() - viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2") + viper.Set("Origin.HttpServiceUrl", srv.URL) - config.InitConfig() fed := fed_test_utils.NewFedTest(t, httpsOriginConfig) - host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt()) + storageName = fed.Exports[0].StoragePrefix + "/hello_world" + discoveryHost := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt()) - _, err = client.DoList(fed.Ctx, "pelican://"+host+"/test/hello_world") + _, err := client.DoList(fed.Ctx, "pelican://"+discoveryHost+"/my-prefix/hello_world") require.Error(t, err) require.Contains(t, err.Error(), "405: object listings are not supported by the discovered origin") } diff --git a/client/resources/test-https-origin.yml b/client/resources/test-https-origin.yml index b38646f9e..31e53842e 100644 --- a/client/resources/test-https-origin.yml +++ b/client/resources/test-https-origin.yml @@ -4,5 +4,6 @@ Logging: Scitokens: debug Origin: StorageType: https - FederationPrefix: /test + FederationPrefix: /my-prefix + StoragePrefix: /test EnablePublicReads: true diff --git a/cmd/plugin_test.go b/cmd/plugin_test.go index 6e1cbd56b..fab544c46 100644 --- a/cmd/plugin_test.go +++ b/cmd/plugin_test.go @@ -455,19 +455,19 @@ func TestPluginDirectRead(t *testing.T) { // We ran into a bug where the start time for the transfer was not recorded correctly and was almost always the same as the end time // (since they were set at similar sections of code). This test ensures that they are different and that the start time is before the end time. func TestPluginCorrectStartAndEndTime(t *testing.T) { - test_utils.InitClient(t, nil) server_utils.ResetOriginExports() defer viper.Reset() defer server_utils.ResetOriginExports() + var storageName string // Set up our http backend so that we can sleep during transfer body := "Hello, World!" srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" { + if r.Method == "HEAD" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusOK) return - } else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" { + } else if r.Method == "GET" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusPartialContent) time.Sleep(1 * time.Second) @@ -478,20 +478,19 @@ func TestPluginCorrectStartAndEndTime(t *testing.T) { w.WriteHeader(http.StatusNotFound) })) defer srv.Close() - viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2") - - config.InitConfig() - tmpPath := t.TempDir() + viper.Set("Origin.HttpServiceUrl", srv.URL) fed := fed_test_utils.NewFedTest(t, httpsOriginConfig) - host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt()) + storageName = fed.Exports[0].StoragePrefix + "/hello_world" + discoveryHost := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt()) downloadUrl := url.URL{ Scheme: "pelican", - Host: host, - Path: "/test/hello_world", + Host: discoveryHost, + Path: "/my-prefix/hello_world", } + tmpPath := t.TempDir() workChan := make(chan PluginTransfer, 2) workChan <- PluginTransfer{url: &downloadUrl, localFile: tmpPath} close(workChan) diff --git a/cmd/resources/test-https-origin.yml b/cmd/resources/test-https-origin.yml index 14a3491d4..2dd4ee1bc 100644 --- a/cmd/resources/test-https-origin.yml +++ b/cmd/resources/test-https-origin.yml @@ -3,5 +3,6 @@ Logging: Scitokens: debug Origin: StorageType: https - FederationPrefix: /test + FederationPrefix: /my-prefix + StoragePrefix: /test EnablePublicReads: true diff --git a/xrootd/fed_test.go b/xrootd/fed_test.go index 82bae9795..3ed9773ec 100644 --- a/xrootd/fed_test.go +++ b/xrootd/fed_test.go @@ -24,24 +24,18 @@ import ( _ "embed" "net/http" "net/http/httptest" - "net/url" - "os" "path/filepath" "strconv" "testing" - "time" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/pelicanplatform/pelican/client" - "github.com/pelicanplatform/pelican/config" "github.com/pelicanplatform/pelican/fed_test_utils" - "github.com/pelicanplatform/pelican/server_structs" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_utils" - "github.com/pelicanplatform/pelican/token" - "github.com/pelicanplatform/pelican/token_scopes" ) var ( @@ -50,19 +44,20 @@ var ( ) func TestHttpOriginConfig(t *testing.T) { - viper.Reset() - viper.Set("ConfigDir", t.TempDir()) server_utils.ResetOriginExports() defer viper.Reset() defer server_utils.ResetOriginExports() + // temp place holder so we can start the test server before this value has been parsed + // from the http origin config + var storageName string body := "Hello, World!" srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" { + if r.Method == "HEAD" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusOK) return - } else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" { + } else if r.Method == "GET" && r.URL.Path == storageName { w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(http.StatusPartialContent) _, err := w.Write([]byte(body)) @@ -72,59 +67,19 @@ func TestHttpOriginConfig(t *testing.T) { w.WriteHeader(http.StatusNotFound) })) defer srv.Close() - - modules := server_structs.ServerType(0) - modules.Set(server_structs.OriginType) - modules.Set(server_structs.DirectorType) - modules.Set(server_structs.RegistryType) - - viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2") - viper.Set("Origin.FederationPrefix", "/test") - - config.InitConfig() - - tmpPath := t.TempDir() + viper.Set("Origin.HttpServiceUrl", srv.URL) fed := fed_test_utils.NewFedTest(t, httpsOriginConfig) - - issuer, err := config.GetServerIssuerURL() - require.NoError(t, err) - - // Create a token file - tokenConfig := token.NewWLCGToken() - tokenConfig.Lifetime = time.Minute - tokenConfig.Issuer = issuer - tokenConfig.Subject = "origin" - tokenConfig.AddAudienceAny() - - scopes := []token_scopes.TokenScope{} - readScope, err := token_scopes.Storage_Read.Path("/") - assert.NoError(t, err) - scopes = append(scopes, readScope) - modScope, err := token_scopes.Storage_Modify.Path("/") - assert.NoError(t, err) - scopes = append(scopes, modScope) - tokenConfig.AddScopes(scopes...) - token, err := tokenConfig.CreateToken() - assert.NoError(t, err) - tempToken, err := os.CreateTemp(t.TempDir(), "token") - assert.NoError(t, err, "Error creating temp token file") - defer tempToken.Close() - _, err = tempToken.WriteString(token) - assert.NoError(t, err, "Error writing to temp token file") - - fedInfo, err := config.GetFederation(fed.Ctx) - require.NoError(t, err) - fedUrl, err := url.Parse(fedInfo.DirectorEndpoint) - require.NoError(t, err) + storageName = fed.Exports[0].StoragePrefix + "/hello_world" + discoveryHost := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt()) // Download the test file + tmpPath := t.TempDir() transferResults, err := client.DoGet( fed.Ctx, - "pelican://"+fedUrl.Host+"/test/hello_world", + "pelican://"+discoveryHost+"/my-prefix/hello_world", filepath.Join(tmpPath, "hw"), false, - client.WithTokenLocation(tempToken.Name()), ) assert.NoError(t, err) if err == nil { diff --git a/xrootd/resources/test-https-origin.yml b/xrootd/resources/test-https-origin.yml index f1bbece87..2dd4ee1bc 100644 --- a/xrootd/resources/test-https-origin.yml +++ b/xrootd/resources/test-https-origin.yml @@ -3,4 +3,6 @@ Logging: Scitokens: debug Origin: StorageType: https - FederationPrefix: /test + FederationPrefix: /my-prefix + StoragePrefix: /test + EnablePublicReads: true