From 2bdf75ab04da7550dab96bdf2b970a105415a2c2 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 29 Apr 2026 09:36:20 +0100 Subject: [PATCH 1/2] fix(domain): allow null --attach in domain:add on production env EnvironmentDomain::add declared $replacementFor as a non-nullable string with default '', but DomainCommandBase::$attach is ?string defaulting to null. Under strict_types this raised a TypeError at the call site when domain:add was run on a production environment without --attach. Make $replacementFor nullable. The existing !empty() guard already treats null and '' the same, so the request body is unchanged. Add an integration test that runs domain:add against a mocked API and asserts the captured request body contains the domain name without replacement_for. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/domain_add_test.go | 69 ++++++++++++++++++++++++++ legacy/src/Model/EnvironmentDomain.php | 2 +- 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 integration-tests/domain_add_test.go diff --git a/integration-tests/domain_add_test.go b/integration-tests/domain_add_test.go new file mode 100644 index 000000000..b0c8f55a0 --- /dev/null +++ b/integration-tests/domain_add_test.go @@ -0,0 +1,69 @@ +package tests + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestDomainAdd is a regression test for a TypeError raised by the legacy +// domain:add command on a production environment when --attach was not given. +func TestDomainAdd(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + "#manage-domains=/projects/"+projectID+"/domains", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#domains"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/domains"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "custom_domains": map[string]any{"enabled": true}, + }) + }) + + type createRequest struct { + Name string `json:"name"` + ReplacementFor string `json:"replacement_for"` + } + var captured createRequest + apiHandler.Post("/projects/"+projectID+"/environments/main/domains", func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(body, &captured)) + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{"name": captured.Name}) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput("domain:add", "-p", projectID, "-e", ".", "--no-wait", "example.com") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "Adding the domain: example.com") + assert.Equal(t, "example.com", captured.Name) + assert.Empty(t, captured.ReplacementFor) +} diff --git a/legacy/src/Model/EnvironmentDomain.php b/legacy/src/Model/EnvironmentDomain.php index 9bbe27589..60764ac55 100644 --- a/legacy/src/Model/EnvironmentDomain.php +++ b/legacy/src/Model/EnvironmentDomain.php @@ -33,7 +33,7 @@ public static function getList(Environment $environment, ClientInterface $client * * @param array $ssl */ - public static function add(ClientInterface $client, Environment $environment, string $name, string $replacementFor = '', array $ssl = []): Result + public static function add(ClientInterface $client, Environment $environment, string $name, ?string $replacementFor = null, array $ssl = []): Result { $body = ['name' => $name]; if (!empty($replacementFor)) { From 09e5bda767778ee77793fecb5b29bd27caebe00f Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 29 Apr 2026 09:50:16 +0100 Subject: [PATCH 2/2] test(domain): assert replacement_for absent in request body Decode the captured POST body into map[string]any and assert the replacement_for key is not present. Decoding into a string field collapsed absent, "" and null to the same value, so the previous assertion did not actually verify the regression. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/domain_add_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/integration-tests/domain_add_test.go b/integration-tests/domain_add_test.go index b0c8f55a0..3cd6b0a02 100644 --- a/integration-tests/domain_add_test.go +++ b/integration-tests/domain_add_test.go @@ -43,17 +43,13 @@ func TestDomainAdd(t *testing.T) { }) }) - type createRequest struct { - Name string `json:"name"` - ReplacementFor string `json:"replacement_for"` - } - var captured createRequest + var captured map[string]any apiHandler.Post("/projects/"+projectID+"/environments/main/domains", func(w http.ResponseWriter, req *http.Request) { body, err := io.ReadAll(req.Body) require.NoError(t, err) require.NoError(t, json.Unmarshal(body, &captured)) w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(map[string]any{"name": captured.Name}) + _ = json.NewEncoder(w).Encode(map[string]any{"name": captured["name"]}) }) apiServer := httptest.NewServer(apiHandler) @@ -64,6 +60,6 @@ func TestDomainAdd(t *testing.T) { stdout, stderr, err := f.RunCombinedOutput("domain:add", "-p", projectID, "-e", ".", "--no-wait", "example.com") require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) assert.Contains(t, stderr, "Adding the domain: example.com") - assert.Equal(t, "example.com", captured.Name) - assert.Empty(t, captured.ReplacementFor) + assert.Equal(t, "example.com", captured["name"]) + assert.NotContains(t, captured, "replacement_for", "request body must omit replacement_for when --attach is not given") }