refactor: require explicit gateway URLs

This is important UX change. We no longer ship with default URLs.

User has to provide explicit one, or the test suite will refuse to run.
This ensures misconfigurations and testing different gateway endpoint
than desired do not happen.

Explicit is better than implicit.

CHANGELOG.md

@@ -4,18 +4,33 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.6.0] - 2024-05-27
-### Changed
-- Generic tests
-  - The `--gateway-url` now defaults to `http://127.0.0.1:8080` to ensure no confusion with subdomain gateway feature.
-- Subdomain tests
-  - The `--subdomain-url` now defaults to `http://example.com:8080`
-  - We also simplified the way `--subdomain-url` works. We no longer run implicit tests
+## [0.6.0] - 2024-05-27 ### Changed
+- Gateway URL
+  - `--gateway-url` is no longer defaulting to predefined URL. User has to
+    provide it via CLI or `GATEWAY_URL` environment variable or the test suite
+    will refuse to start.
+  - This aims to ensure no confusion about which gateway endpoint is being
+    tested.
+  - Docs and examples use `--gateway-url http://127.0.0.1:8080` to ensure no
+    confusion with `localhost:8080` subdomain gateway feature in IPFS
+    implementations like Kubo.
+- Subdomain URL and UX related to subdomain tests
+  - The `--subdomain-url` is no longer set by default.
+  - User has to provide the origin of the subdomain gateway via CLI or
+    `SUBDOMAIN_GATEWAY_URL` to be used during subdomain tests. This aims to
+    ensure no confusion about which domain name is being tested.
+  - Simplified the way `--subdomain-url` works. We no longer run implicit tests
     against `http://localhost` in addition to the URL passed via
     `--subdomain-url`. To test more than one domain, run test multiple times.
--  DNSLink tests
-  - DNSLink fixtures no longer depend on `--subdomain-url` and use unrelated `*.example.org` domains instead.
-  - `gateway-conformance extract-fixtures` creates `dnslinks.IPFS_NS_MAP` with content that can be directly set as `IPNS_NS_MAP` environment variable supported by various implementations, incl. [Kubo](https://github.com/ipfs/kubo/blob/master/docs/environment-variables.md#ipfs_ns_map) and [Rainbow](https://github.com/ipfs/rainbow/blob/main/docs/environment-variables.md#ipfs_ns_map).
+-  DNSLink test fixtures changed
+  - DNSLink fixtures no longer depend on `--subdomain-url` and use unrelated
+    `*.example.org` domains instead.
+  - `gateway-conformance extract-fixtures` creates `dnslinks.IPFS_NS_MAP` with
+    content that can be directly set as `IPNS_NS_MAP` environment variable
+    supported by various implementations, incl.
+    [Kubo](https://github.com/ipfs/kubo/blob/master/docs/environment-variables.md#ipfs_ns_map)
+    and
+    [Rainbow](https://github.com/ipfs/rainbow/blob/main/docs/environment-variables.md#ipfs_ns_map).
 - Docker
   - The image can now be run under non-root user
 

cmd/gateway-conformance/main.go

@@ -85,13 +85,13 @@ func main() {
 						EnvVars: []string{"GATEWAY_URL"},
 						Aliases: []string{"url", "g"},
 						Usage:   "The URL of the IPFS Gateway implementation to be tested.",
-						Value:   "http://127.0.0.1:8080",
+						Value:   "", // unset by default, requires end user to either provide configured gateway endpoint URL
 					},
 					&cli.StringFlag{
 						Name:    "subdomain-url",
 						EnvVars: []string{"SUBDOMAIN_GATEWAY_URL"},
 						Usage:   "URL of the HTTP Host that should be used when testing https://specs.ipfs.tech/http-gateways/subdomain-gateway/ functionality",
-						Value:   "http://example.com:8080",
+						Value:   "", // unset by default, requires end user to either provide configured subdomain gateway origin URL, or pass '--specs -subdomain-gateway' to disable these tests
 					},
 					&cli.StringFlag{
 						Name:    "json-output",
@@ -120,27 +120,37 @@ func main() {
 				Action: func(cctx *cli.Context) error {
 					env := os.Environ()
 					verbose := cctx.Bool("verbose")
+					specs := cctx.String("specs")
 
-					// Set gateway URLs
+					// Handle Gateway Endpoint URL
 					gatewayURL := cctx.String("gateway-url")
-					subdomainGatewayURL := cctx.String("subdomain-url")
-					envGwURL := fmt.Sprintf("GATEWAY_URL=%s", gatewayURL)
-					if verbose {
-						fmt.Println(envGwURL)
+					if gatewayURL != "" {
+						envGwURL := fmt.Sprintf("GATEWAY_URL=%s", gatewayURL)
+						if verbose {
+							fmt.Println(envGwURL)
+						}
+						env = append(env, envGwURL)
+					} else {
+						return cli.Exit("⚠️ GATEWAY_URL (or --gateway-url) with the endpoint to receive HTTP requests has to be set", 2)
 					}
-					env = append(env, envGwURL)
+
+					// Handle Subdomain URL
+					subdomainGatewayURL := cctx.String("subdomain-url")
 					if subdomainGatewayURL != "" {
+						// If set, pass to `go test` via env
 						envSubdomainGwURL := fmt.Sprintf("SUBDOMAIN_GATEWAY_URL=%s", subdomainGatewayURL)
 						if verbose {
 							fmt.Println(envSubdomainGwURL)
 						}
 						env = append(env, envSubdomainGwURL)
+					} else if isSubdomainPresetEnabled(specs) {
+						// If not set, check if `specs` is not set to explicitly disable it,
+						// provide user with a meaningful error
+						return cli.Exit("⚠️ SUBDOMAIN_GATEWAY_URL (or --subdomain-url) must be set when 'subdomain-gateway' tests are enabled. Set the URL and try again, or disable related tests by passing --specs -subdomain-gateway", 2)
 					}
 
 					// Set other parameters
 					args := []string{"test", "./tests", "-test.v=test2json"}
-
-					specs := cctx.String("specs")
 					if specs != "" {
 						args = append(args, fmt.Sprintf("-specs=%s", specs))
 					}
@@ -345,3 +355,31 @@ func getAvailableSpecPresets() []string {
 	}
 	return presets
 }
+
+func isSubdomainPresetEnabled(specs string) bool {
+	isEnabledByDefault := specPresets.SubdomainGateway.IsEnabled()
+	if specs == "" && isEnabledByDefault {
+		return true
+	}
+	subdomainSpec := specPresets.SubdomainGateway.Name()
+	userProvidedSpecsList := strings.Split(specs, ",")
+	manualList := false // did user set --specs to at least one without the -/+ prefix
+	for _, s := range userProvidedSpecsList {
+		// Return early if user-provided spec entry is one that controls subdomain gateway tests
+		if s == "-"+subdomainSpec {
+			return false
+		}
+		if strings.HasSuffix(s, subdomainSpec) {
+			return true // at this point  it can be + or manual entry
+		}
+		// Subdomain gateway preset is implicitly enabled, but it gets disabled
+		// if user explicitly enabled other one (without - or + prefix)
+		if !strings.HasPrefix(s, "-") && !strings.HasPrefix(s, "+") {
+			manualList = true
+		}
+	}
+	// at this point, if the list was manual, and we did not return yet,
+	// subdomain preset is enabled only if user-provided list had no explicit entries
+	// (empty or only with -/+ entries)
+	return !manualList
+}

cmd/gateway-conformance/main_test.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"testing"
+)
+
+// Test cases for isSubdomainPresetEnabled function
+func TestIsSubdomainPresetEnabled(t *testing.T) {
+	tests := []struct {
+		specs           string
+		expectedEnabled bool
+		description     string
+	}{
+		// Test case 1: Empty specs and subdomain preset is enabled by default
+		{
+			specs:           "",
+			expectedEnabled: true,
+			description:     "Empty specs and subdomain preset is enabled by default",
+		},
+		// Test case 2: User provides "-subdomain", should be disabled explicitly
+		{
+			specs:           "-subdomain-gateway",
+			expectedEnabled: false,
+			description:     "User provides '-subdomain', should be disabled explicitly",
+		},
+		// Test case 3: User provides "+subdomain", should be enabled explicitly
+		{
+			specs:           "+subdomain-gateway",
+			expectedEnabled: true,
+			description:     "User provides '+subdomain', should be enabled explicitly",
+		},
+		// Test case 4: User provides "+other", should not affect subdomain preset
+		{
+			specs:           "+proxy-gateway",
+			expectedEnabled: true,
+			description:     "User provides '+proxy-gateway', should not affect subdomain-gateway preset default",
+		},
+		// Test case 5: User provides "other", subdomain preset should be enabled by default
+		{
+			specs:           "path-gateway",
+			expectedEnabled: false,
+			description:     "User provides 'path-gateway', subdomain preset should be disabled due to explicit (manual) list",
+		},
+		// Test case 6: User provides "-other,+subdomain", should be enabled due to +subdomain
+		{
+			specs:           "-path-gateway,+subdomain-gateway",
+			expectedEnabled: true,
+			description:     "User provides '-path-gateway,+subdomain-gateway', should be enabled due to +subdomain-gateway",
+		},
+		// Test case 7: User provides "+other,-subdomain", should be disabled due to -subdomain
+		{
+			specs:           "+path-gateway,-subdomain-gateway",
+			expectedEnabled: false,
+			description:     "User provides '+path-gateway,-subdomain-gateway', should be disabled due to -subdomain-gateway",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			actualEnabled := isSubdomainPresetEnabled(test.specs)
+			if actualEnabled != test.expectedEnabled {
+				t.Errorf("Expected isSubdomainPresetEnabled(%q) to be %v, but got %v",
+					test.specs, test.expectedEnabled, actualEnabled)
+			}
+		})
+	}
+}

tooling/test/config.go

@@ -13,7 +13,10 @@ var log = logging.Logger("conformance")
 func env2url(key string) *url.URL {
 	value, ok := os.LookupEnv(key)
 	if !ok {
-		panic(key + " must be set")
+		// Actual validation of GATEWAY_URL and SUBDOMAIN_GATEWAY_URL happens in
+		// in cmd/gateway-conformance/main.go. Here we create a bogus URL
+		// to ensure no error at compilation time.
+		value = "http://todo-changeme.example.com"
 	}
 	gatewayURL := strings.TrimRight(value, "/")
 	parsed, err := url.Parse(gatewayURL)