fix(provx/gcp): trust the issuer the caller names, not the one compiled in

cmd/oox/cli/provision_gcp.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"time"
 
+	"github.com/platform-engineering-labs/oox/provx"
 	"github.com/platform-engineering-labs/oox/provx/gcp"
 	"github.com/spf13/cobra"
 )
@@ -48,7 +49,7 @@ var ProvisionGCPCreate = &cobra.Command{
 		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 		defer cancel()
 
-		prov, err := gcp.New(ctx, slog.New(Logger), project, tenantId, installationId)
+		prov, err := gcp.New(ctx, slog.New(Logger), project, tenantId, installationId, "https://"+provx.Endpoint)
 		if err != nil {
 			return err
 		}
@@ -87,7 +88,7 @@ var ProvisionGCPDelete = &cobra.Command{
 		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 		defer cancel()
 
-		prov, err := gcp.New(ctx, slog.New(Logger), project, tenantId, installationId)
+		prov, err := gcp.New(ctx, slog.New(Logger), project, tenantId, installationId, "https://"+provx.Endpoint)
 		if err != nil {
 			return err
 		}

provx/aws/aws_test.go

@@ -21,7 +21,12 @@ func (f *fakeSTS) GetCallerIdentity(ctx context.Context, in *sts.GetCallerIdenti
 	return &sts.GetCallerIdentityOutput{Account: &f.account, Arn: &f.arn}, nil
 }
 
-const testIssuer = "https://oidc.cloud.formae.ai"
+// testIssuer is deliberately not the production issuer. The production code
+// takes the issuer as a parameter, and the assertions below pin it as it
+// appears in the provider URL and the trust policy — but written with the
+// production value they would pass just as well against an issuer resolved
+// from provx.Endpoint, which is the regression they exist to catch.
+const testIssuer = "https://issuer.test.example"
 
 func TestNewVerifiesAccount(t *testing.T) {
 	_, err := newWithClients(context.Background(),

provx/aws/connect_provider_test.go

@@ -33,7 +33,7 @@ func TestEnsureProviderCreates(t *testing.T) {
 	if err != nil || out != ProviderCreated {
 		t.Fatalf("out=%v err=%v", out, err)
 	}
-	if gotURL != "https://oidc.cloud.formae.ai" || len(gotClients) != 1 || gotClients[0] != "sts.amazonaws.com" {
+	if gotURL != "https://issuer.test.example" || len(gotClients) != 1 || gotClients[0] != "sts.amazonaws.com" {
 		t.Fatalf("create input: url=%q clients=%v", gotURL, gotClients)
 	}
 }
@@ -45,7 +45,7 @@ func TestEnsureProviderExistsValid(t *testing.T) {
 		},
 		getOIDCProvider: func(in *iam.GetOpenIDConnectProviderInput) (*iam.GetOpenIDConnectProviderOutput, error) {
 			return &iam.GetOpenIDConnectProviderOutput{
-				Url:          awssdk.String("oidc.cloud.formae.ai"),
+				Url:          awssdk.String("issuer.test.example"),
 				ClientIDList: []string{"sts.amazonaws.com"},
 			}, nil
 		},
@@ -64,7 +64,7 @@ func TestEnsureProviderAddsClientID(t *testing.T) {
 			return nil, &types.EntityAlreadyExistsException{}
 		},
 		getOIDCProvider: func(*iam.GetOpenIDConnectProviderInput) (*iam.GetOpenIDConnectProviderOutput, error) {
-			return &iam.GetOpenIDConnectProviderOutput{Url: awssdk.String("oidc.cloud.formae.ai"), ClientIDList: []string{"other"}}, nil
+			return &iam.GetOpenIDConnectProviderOutput{Url: awssdk.String("issuer.test.example"), ClientIDList: []string{"other"}}, nil
 		},
 		addClientID: func(in *iam.AddClientIDToOpenIDConnectProviderInput) (*iam.AddClientIDToOpenIDConnectProviderOutput, error) {
 			if awssdk.ToString(in.ClientID) != "sts.amazonaws.com" {

provx/aws/create_test.go

@@ -52,7 +52,7 @@ func TestCreateRerunConverges(t *testing.T) {
 		},
 		getOIDCProvider: func(*iam.GetOpenIDConnectProviderInput) (*iam.GetOpenIDConnectProviderOutput, error) {
 			return &iam.GetOpenIDConnectProviderOutput{
-				Url:          awssdk.String("oidc.cloud.formae.ai"),
+				Url:          awssdk.String("issuer.test.example"),
 				ClientIDList: []string{"sts.amazonaws.com"},
 			}, nil
 		},

provx/aws/role_test.go

@@ -17,7 +17,7 @@ import (
 // policy by unmarshal-and-compare, not byte equality.
 func assertSemanticTrustDoc(t *testing.T, doc string) {
 	t.Helper()
-	want := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::111122223333:oidc-provider/oidc.cloud.formae.ai"},"Action":"sts:AssumeRoleWithWebIdentity","Condition":{"StringEquals":{"oidc.cloud.formae.ai:aud":"sts.amazonaws.com","oidc.cloud.formae.ai:sub":"fai:t/i"}}}]}`
+	want := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::111122223333:oidc-provider/issuer.test.example"},"Action":"sts:AssumeRoleWithWebIdentity","Condition":{"StringEquals":{"issuer.test.example:aud":"sts.amazonaws.com","issuer.test.example:sub":"fai:t/i"}}}]}`
 	var gotV, wantV any
 	if err := json.Unmarshal([]byte(doc), &gotV); err != nil {
 		t.Fatalf("trust policy does not parse: %v\n%s", err, doc)

provx/gcp/fake_test.go

@@ -238,20 +238,32 @@ func errorResponse(e *googleError) (*http.Response, error) {
 	return jsonResponse(e.status, payload)
 }
 
+// testIssuer is the issuer the fake-backed provisioners trust. It is not the
+// production one on purpose: an issuer compiled in rather than taken from the
+// caller would pass every test that used the production value.
+const testIssuer = "https://issuer.test.example"
+
 // newTestGCP builds a GCP provisioner wired to the fake.
 func newTestGCP(t *testing.T, f *fakeGoogle, tenant, installation string) *GCP {
 	t.Helper()
-	g, err := New(t.Context(), discardLogger(), "test-project", tenant, installation,
-		option.WithoutAuthentication(),
-		option.WithEndpoint("https://example.invalid/"),
-		option.WithHTTPClient(&http.Client{Transport: f}),
-	)
+	g, err := newTestGCPWithIssuer(t, f, tenant, installation, testIssuer)
 	if err != nil {
 		t.Fatalf("New: %v", err)
 	}
 	return g
 }
 
+// newTestGCPWithIssuer is newTestGCP with the issuer left to the caller and
+// the construction error returned, for the cases that are about the issuer.
+func newTestGCPWithIssuer(t *testing.T, f *fakeGoogle, tenant, installation, issuer string) (*GCP, error) {
+	t.Helper()
+	return New(t.Context(), discardLogger(), "test-project", tenant, installation, issuer,
+		option.WithoutAuthentication(),
+		option.WithEndpoint("https://example.invalid/"),
+		option.WithHTTPClient(&http.Client{Transport: f}),
+	)
+}
+
 func mustCreate(t *testing.T, g *GCP) *Result {
 	t.Helper()
 	res, err := g.Create(t.Context())

provx/gcp/gcp.go

@@ -50,16 +50,29 @@ type GCP struct {
 	project        string
 	tenantId       string
 	installationId string
+	issuer         provx.Issuer
 }
 
 // New builds a provisioner for one installation's connection to one project.
 //
+// issuer is the outbound identity issuer the provider will trust, taken from
+// the caller and validated here rather than compiled in. The AWS provisioner
+// has always worked this way, and the reason is the same on both clouds: the
+// issuer is produced by the control plane and travels verbatim into the trust
+// artifacts, so a build that substituted its own would provision trust for an
+// issuer the caller never named.
+//
 // The context is the caller's: it bounds the client construction, and the
 // credential lookup that construction performs. opts are passed through to the
 // underlying Google clients, which is how tests reach a fake without a network
 // or a credential.
-func New(ctx context.Context, logger *slog.Logger, project, tenantId, installationId string,
+func New(ctx context.Context, logger *slog.Logger, project, tenantId, installationId, issuer string,
 	opts ...option.ClientOption) (*GCP, error) {
+	iss, err := provx.ParseIssuer(issuer)
+	if err != nil {
+		return nil, err
+	}
+
 	client, err := provider.NewClient(ctx, opts...)
 	if err != nil {
 		return nil, err
@@ -71,6 +84,7 @@ func New(ctx context.Context, logger *slog.Logger, project, tenantId, installati
 		project:        project,
 		tenantId:       tenantId,
 		installationId: installationId,
+		issuer:         iss,
 	}, nil
 }
 
@@ -181,12 +195,12 @@ func (gcp *GCP) assertProviderIsOurs(ctx context.Context, pool provider.PoolSpec
 	if existing == nil || existing.Oidc == nil {
 		return nil // nothing there yet, or nothing to disagree with
 	}
-	if existing.Oidc.IssuerUri == issuerURI() {
+	if existing.Oidc.IssuerUri == gcp.issuer.URL() {
 		return nil
 	}
 	return &ProviderNotOursError{
 		Name:          existing.Name,
-		IssuerWanted:  issuerURI(),
+		IssuerWanted:  gcp.issuer.URL(),
 		IssuerFound:   existing.Oidc.IssuerUri,
 		PoolID:        poolID,
 		ProviderID:    providerID,
@@ -207,7 +221,7 @@ func (gcp *GCP) spec(projectNumber string) (*provider.PoolSpec, *provider.OIDCPr
 	oidcProvider := &provider.OIDCProviderSpec{
 		ProviderID:  providerID,
 		DisplayName: "formae ai Cloud OIDC",
-		IssuerURI:   issuerURI(),
+		IssuerURI:   gcp.issuer.URL(),
 		AttributeMapping: map[string]string{
 			"google.subject": "assertion.sub",
 		},
@@ -222,8 +236,6 @@ func (gcp *GCP) spec(projectNumber string) (*provider.PoolSpec, *provider.OIDCPr
 	return pool, oidcProvider
 }
 
-func issuerURI() string { return fmt.Sprintf("https://%s", provx.Endpoint) }
-
 // subjectNamespaceCondition is the CEL expression Google evaluates against an
 // incoming assertion. It admits every subject this issuer mints and nothing
 // else.

provx/gcp/gcp_test.go

@@ -200,6 +200,37 @@ func TestCreateReturnsTheProviderName(t *testing.T) {
 	}
 }
 
+// TestProviderTrustsTheIssuerItWasGiven holds the issuer to the caller's. It
+// is the whole reason New takes one: an issuer resolved inside this package
+// would provision trust for whatever this build was compiled against, no
+// matter which issuer the control plane named.
+func TestProviderTrustsTheIssuerItWasGiven(t *testing.T) {
+	f := newFakeGoogle()
+	mustCreate(t, newTestGCP(t, f, "tenant-a", "install-1"))
+
+	for _, p := range f.createdProviders {
+		if p.Oidc.IssuerUri != testIssuer {
+			t.Errorf("provider issuerUri = %q, want %q", p.Oidc.IssuerUri, testIssuer)
+		}
+	}
+	if len(f.createdProviders) == 0 {
+		t.Fatal("no provider was created")
+	}
+}
+
+// TestRefusesAnIssuerThatIsNotACanonicalOrigin fails the construction rather
+// than the provisioning: an issuer with a path or a port cannot be compared
+// against what Google stores, so accepting one here would defer the failure to
+// a provider that silently never matches.
+func TestRefusesAnIssuerThatIsNotACanonicalOrigin(t *testing.T) {
+	f := newFakeGoogle()
+	for _, issuer := range []string{"", "http://issuer.test.example", "https://issuer.test.example/", "https://issuer.test.example:8443"} {
+		if _, err := newTestGCPWithIssuer(t, f, "tenant-a", "install-1", issuer); err == nil {
+			t.Errorf("New accepted issuer %q", issuer)
+		}
+	}
+}
+
 // TestRefusesAProviderThatIsNotOurs guards the shared singleton: the pool and
 // provider ids are fixed, so an object of that name belonging to a different
 // deployment must not be adopted and rewritten.