From 197169e5feec0b0b67a3a5d8e8a769cecbfcd1c4 Mon Sep 17 00:00:00 2001 From: Sebastian Rivera Date: Mon, 10 Aug 2026 17:21:17 -0400 Subject: [PATCH 1/2] feat: cloud backend query + policy view updates --- go.mod | 8 + go.sum | 16 + internal/cloud/backend.go | 30 ++ internal/cloud/backend_query.go | 108 ++++- internal/cloud/backend_query_test.go | 433 ++++++++++++++++++ .../cloud/testdata/query-json-policy/main.tf | 3 + .../query-json-policy/main.tfquery.hcl | 3 + .../testdata/query-json-policy/query.log | 5 + internal/cloud/testing.go | 76 ++- internal/cloud/tfe_client_mock.go | 4 + internal/command/views/json_view.go | 2 +- internal/command/views/query_operation.go | 20 +- internal/command/views/query_policy.go | 68 ++- internal/command/views/query_policy_test.go | 153 +++++-- 14 files changed, 861 insertions(+), 68 deletions(-) create mode 100644 internal/cloud/testdata/query-json-policy/main.tf create mode 100644 internal/cloud/testdata/query-json-policy/main.tfquery.hcl create mode 100644 internal/cloud/testdata/query-json-policy/query.log diff --git a/go.mod b/go.mod index d4438d4a6d74..3b0fe08d2899 100644 --- a/go.mod +++ b/go.mod @@ -195,6 +195,7 @@ require ( github.com/hashicorp/go-metrics v0.5.4 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-rootcerts v1.0.2 // indirect + github.com/hashicorp/go-tfe/v2 v2.6.0 github.com/hashicorp/golang-lru v1.0.2 // indirect github.com/hashicorp/logutils v1.0.0 // indirect github.com/hashicorp/serf v0.10.2 // indirect @@ -228,6 +229,12 @@ require ( github.com/mattn/go-runewidth v0.0.16 // indirect github.com/mergestat/timediff v0.0.4 // indirect github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect + github.com/microsoft/kiota-abstractions-go v1.9.4 // indirect + github.com/microsoft/kiota-http-go v1.5.6 // indirect + github.com/microsoft/kiota-serialization-form-go v1.1.3 // indirect + github.com/microsoft/kiota-serialization-json-go v1.1.2 // indirect + github.com/microsoft/kiota-serialization-multipart-go v1.1.2 // indirect + github.com/microsoft/kiota-serialization-text-go v1.1.3 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-testing-interface v1.14.1 // indirect github.com/mitchellh/iochan v1.0.0 // indirect @@ -255,6 +262,7 @@ require ( github.com/spf13/cobra v1.10.2 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/spiffe/go-spiffe/v2 v2.6.0 // indirect + github.com/std-uritemplate/std-uritemplate/go/v2 v2.0.10 // indirect github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.588 // indirect github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/sts v1.0.588 // indirect github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/tag v1.0.233 // indirect diff --git a/go.sum b/go.sum index 03d7bc48706a..7371de410d01 100644 --- a/go.sum +++ b/go.sum @@ -380,6 +380,8 @@ github.com/hashicorp/go-sockaddr v1.0.5 h1:dvk7TIXCZpmfOlM+9mlcrWmWjw/wlKT+VDq2w github.com/hashicorp/go-sockaddr v1.0.5/go.mod h1:uoUUmtwU7n9Dv3O4SNLeFvg0SxQ3lyjsj6+CCykpaxI= github.com/hashicorp/go-tfe v1.110.0 h1:R61zw8hgXH+A06rb77GhZXkexjiTls/sPM+lL3G4VsE= github.com/hashicorp/go-tfe v1.110.0/go.mod h1:VH4URSfSw6421VEBdfjub/oTINTvT5Mhp4Gd9IA3Ifw= +github.com/hashicorp/go-tfe/v2 v2.6.0 h1:1CItfvWIAE09qLr644qDkzl0KZBv08O9Gu7je8CLSK4= +github.com/hashicorp/go-tfe/v2 v2.6.0/go.mod h1:gosuJ9PH3NLxkCoCW3EIeHHli+5QqLUkboBiUZ1ljCM= github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= @@ -513,6 +515,18 @@ github.com/mergestat/timediff v0.0.4/go.mod h1:yvMUaRu2oetc+9IbPLYBJviz6sA7xz8OX github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQE9x6ikvDFZS2mDVS3drnohI= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= +github.com/microsoft/kiota-abstractions-go v1.9.4 h1:VI3UVzSCQHHhRswe3jyaAQHUQWIFhUMp0z5mtZbTbcs= +github.com/microsoft/kiota-abstractions-go v1.9.4/go.mod h1:f06pl3qSyvUHEfVNkiRpXPkafx7khZqQEb71hN/pmuU= +github.com/microsoft/kiota-http-go v1.5.6 h1:KBdk7sxWYXZnRRExLjIcNt4I7LoOfh/XQJWWid4zBKE= +github.com/microsoft/kiota-http-go v1.5.6/go.mod h1:bpJkXfBAcnmiXRg03GXdnb/vF3Sqk3+EgLvXXjmzzQM= +github.com/microsoft/kiota-serialization-form-go v1.1.3 h1:eUY8eHXPFe4ma8cAdx0ya3g4NPlZgbPT+GlFC3xcgGY= +github.com/microsoft/kiota-serialization-form-go v1.1.3/go.mod h1:RMO99zyik+NvZjdVcIeyu6ikyfuKhQtzq2RK0fWJJio= +github.com/microsoft/kiota-serialization-json-go v1.1.2 h1:eJrPWeQ665nbjO0gsHWJ0Bw6V/ZHHU1OfFPaYfRG39k= +github.com/microsoft/kiota-serialization-json-go v1.1.2/go.mod h1:deaGt7fjZarywyp7TOTiRsjfYiyWxwJJPQZytXwYQn8= +github.com/microsoft/kiota-serialization-multipart-go v1.1.2 h1:1pUyA1QgIeKslQwbk7/ox1TehjlCUUT3r1f8cNlkvn4= +github.com/microsoft/kiota-serialization-multipart-go v1.1.2/go.mod h1:j2K7ZyYErloDu7Kuuk993DsvfoP7LPWvAo7rfDpdPio= +github.com/microsoft/kiota-serialization-text-go v1.1.3 h1:8z7Cebn0YAAr++xswVgfdxZjnAZ4GOB9O7XP4+r5r/M= +github.com/microsoft/kiota-serialization-text-go v1.1.3/go.mod h1:NDSvz4A3QalGMjNboKKQI9wR+8k+ih8UuagNmzIRgTQ= github.com/miekg/dns v1.1.56 h1:5imZaSeoRNvpM9SzWNhEcP9QliKiz20/dA2QabIGVnE= github.com/miekg/dns v1.1.56/go.mod h1:cRm6Oo2C8TY9ZS/TqsSrseAcncm74lfK5G+ikN2SWWY= github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ= @@ -635,6 +649,8 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spiffe/go-spiffe/v2 v2.6.0 h1:l+DolpxNWYgruGQVV0xsfeya3CsC7m8iBzDnMpsbLuo= github.com/spiffe/go-spiffe/v2 v2.6.0/go.mod h1:gm2SeUoMZEtpnzPNs2Csc0D/gX33k1xIx7lEzqblHEs= +github.com/std-uritemplate/std-uritemplate/go/v2 v2.0.10 h1:8sNWJ4i8eORl8gu97a8EcpRT/y0oapwjddxrf/fbj7w= +github.com/std-uritemplate/std-uritemplate/go/v2 v2.0.10/go.mod h1:Z5KcoM0YLC7INlNhEezeIZ0TZNYf7WSNO0Lvah4DSeQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 5dae9abda868..6d3470790295 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/cli" tfe "github.com/hashicorp/go-tfe" + tfev2 "github.com/hashicorp/go-tfe/v2" version "github.com/hashicorp/go-version" svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform-svchost/disco" @@ -66,6 +67,11 @@ type Cloud struct { // client is the HCP Terraform or Terraform Enterprise API client. client *tfe.Client + // clientV2 is the go-tfe v2 (Kiota-generated) client, used when features + // such as PolicyPaths require fields not yet in the v1 SDK. It is nil when + // the v2 client cannot be initialised; callers must check before use. + clientV2 *tfev2.Client + // viewHooks implements functions integrating the tfe.Client with the CLI // output. viewHooks views.CloudHooks @@ -355,6 +361,30 @@ func (b *Cloud) Configure(obj cty.Value) tfdiags.Diagnostics { )) return diags } + + // Initialise the v2 Kiota client. Failure is non-fatal; features that + // need it will fall back to the v1 path when clientV2 is nil. + // + // Split the service URL into scheme+host and path so that Enterprise + // servers with a non-standard base path (e.g. /tfe/api/v2/) are + // routed correctly. Passing the full URL as Address causes tfev2 to + // overwrite the path with its default /api/v2, losing the prefix. + // + // The Kiota URL template uses {+baseurl}/resource, so the base path + // must not have a trailing slash or the request URL will gain a double + // slash (e.g. /tfe/api/v2//queries). Strip it here. + v2Headers := cfg.Headers.Clone() + v2cfg := &tfev2.Config{ + Address: tfcService.Scheme + "://" + tfcService.Host, + BasePath: strings.TrimRight(tfcService.Path, "/"), + Token: token, + Headers: v2Headers, + } + if v2client, v2err := tfev2.NewClient(v2cfg); v2err == nil { + b.clientV2 = v2client + } else { + log.Printf("[WARN] cloud: failed to create go-tfe v2 client: %s", v2err) + } } // Read the app name header and if empty, provide a default diff --git a/internal/cloud/backend_query.go b/internal/cloud/backend_query.go index 72dcd7eb1c3b..ecf4761a347c 100644 --- a/internal/cloud/backend_query.go +++ b/internal/cloud/backend_query.go @@ -14,8 +14,10 @@ import ( "time" tfe "github.com/hashicorp/go-tfe" + tfev2models "github.com/hashicorp/go-tfe/v2/api/models" "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/command/jsonformat" + "github.com/hashicorp/terraform/internal/command/views" viewsjson "github.com/hashicorp/terraform/internal/command/views/json" "github.com/hashicorp/terraform/internal/genconfig" "github.com/hashicorp/terraform/internal/terraform" @@ -71,7 +73,12 @@ func (b *Cloud) query(stopCtx, cancelCtx context.Context, op *backendrun.Operati } queryRunOptions.Variables = runVariables - r, err := b.client.QueryRuns.Create(stopCtx, queryRunOptions) + var r *tfe.QueryRun + if len(op.PolicyPaths) > 0 && b.clientV2 != nil { + r, err = b.createQueryRunV2(stopCtx, queryRunOptions, op.PolicyPaths) + } else { + r, err = b.client.QueryRuns.Create(stopCtx, queryRunOptions) + } if err != nil { return &QueryRunResult{}, b.generalError("Failed to create query run", err) } @@ -124,8 +131,8 @@ func (b *Cloud) renderQueryRunLogs(ctx context.Context, op *backendrun.Operation } if next || len(line) > 0 { - log := &jsonformat.JSONLog{} - if err := json.Unmarshal(line, log); err != nil { + jsonLog := &jsonformat.JSONLog{} + if err := json.Unmarshal(line, jsonLog); err != nil { // If we can not parse the line as JSON, we will simply // print the line. This maintains backwards compatibility for // users who do not wish to enable structured output in their @@ -139,17 +146,25 @@ func (b *Cloud) renderQueryRunLogs(ctx context.Context, op *backendrun.Operation // we collect all logs of a list block and output them at once. // This allows us to ensure all messages of a list block are grouped // and indented as in the PostListQuery hook. - switch log.Type { + switch jsonLog.Type { + case jsonformat.LogPolicyQuerySummary: + rendered, renderErr := views.RenderPolicyQuerySummaryFromJSON(line) + if renderErr != nil { + // Malformed record — skip gracefully. + log.Printf("[TRACE] cloud: skipping malformed policy_query_summary record: %v", renderErr) + continue + } + b.renderer.Streams.Println(rendered) case jsonformat.LogListStart: - results[log.ListQueryStart.Address] = make([]*viewsjson.QueryResult, 0) + results[jsonLog.ListQueryStart.Address] = make([]*viewsjson.QueryResult, 0) case jsonformat.LogListResourceFound: - results[log.ListQueryResult.Address] = append(results[log.ListQueryResult.Address], log.ListQueryResult) + results[jsonLog.ListQueryResult.Address] = append(results[jsonLog.ListQueryResult.Address], jsonLog.ListQueryResult) if wantConfig { - configs[log.ListQueryResult.Address] += - fmt.Sprintf("%s\n%s\n\n", log.ListQueryResult.Config, log.ListQueryResult.ImportConfig) + configs[jsonLog.ListQueryResult.Address] += + fmt.Sprintf("%s\n%s\n\n", jsonLog.ListQueryResult.Config, jsonLog.ListQueryResult.ImportConfig) } case jsonformat.LogListComplete: - addr := log.ListQueryComplete.Address + addr := jsonLog.ListQueryComplete.Address identities := make([]string, 0, len(results[addr])) displayNames := make([]string, 0, len(results[addr])) @@ -173,7 +188,7 @@ func (b *Cloud) renderQueryRunLogs(ctx context.Context, op *backendrun.Operation b.renderer.Streams.Println(result.String()) } default: - err := b.renderer.RenderLog(log) + err := b.renderer.RenderLog(jsonLog) if err != nil { return err } @@ -206,6 +221,79 @@ func (b *Cloud) renderQueryRunLogs(ctx context.Context, op *backendrun.Operation return nil } +// createQueryRunV2 creates a query run via the go-tfe v2 SDK so that +// PolicyPaths can be forwarded to the server. It posts to POST /queries +// using the generated QueriesRequestBuilder and maps the response back into +// the tfe.QueryRun that the rest of the cloud backend uses. +func (b *Cloud) createQueryRunV2(ctx context.Context, opts tfe.QueryRunCreateOptions, policyPaths []string) (*tfe.QueryRun, error) { + workspaceID := opts.Workspace.ID + + // Build attributes. + attrs := tfev2models.NewQueries_attributes() + if opts.GenerateConfigOut != nil { + attrs.SetGenerateConfigOut(opts.GenerateConfigOut) + } + sourceVal := tfev2models.TFEAPI_QUERIES_ATTRIBUTES_SOURCE + attrs.SetSource(&sourceVal) + attrs.SetPolicyPaths(policyPaths) + + // Configuration-version relationship. + cvData := tfev2models.NewConfigurationVersionsHasOne_data() + cvID := opts.ConfigurationVersion.ID + cvData.SetId(&cvID) + cvRef := tfev2models.NewConfigurationVersionsHasOne() + cvRef.SetData(cvData) + + // Workspace relationship. + wsData := tfev2models.NewWorkspacesHasOne_data() + wsData.SetId(&workspaceID) + wsRef := tfev2models.NewWorkspacesHasOne() + wsRef.SetData(wsData) + + rels := tfev2models.NewQueries_relationships() + rels.SetConfigurationVersion(cvRef) + rels.SetWorkspace(wsRef) + + data := tfev2models.NewQueries() + data.SetAttributes(attrs) + data.SetRelationships(rels) + + envelope := tfev2models.NewQueriesEnvelope() + envelope.SetData(data) + + resp, err := b.clientV2.API.Queries().Post(ctx, envelope, nil) + if err != nil { + return nil, err + } + if resp == nil { + return nil, fmt.Errorf("empty response from query run create") + } + + qry := resp.GetData() + + // Map the v2 response back into the tfe.QueryRun the rest of the + // cloud backend already knows how to handle. + run := &tfe.QueryRun{} + if qry != nil { + if qry.GetId() != nil { + run.ID = *qry.GetId() + } + if qry.GetAttributes() != nil { + a := qry.GetAttributes() + if a.GetLogReadUrl() != nil { + run.LogReadURL = *a.GetLogReadUrl() + } + if a.GetStatus() != nil { + run.Status = tfe.QueryRunStatus(a.GetStatus().String()) + } + } + } + if run.Status == "" { + run.Status = tfe.QueryRunPending + } + return run, nil +} + func (b *Cloud) waitForQueryRun(stopCtx, cancelCtx context.Context, r *tfe.QueryRun) (*tfe.QueryRun, error) { started := time.Now() updated := started diff --git a/internal/cloud/backend_query_test.go b/internal/cloud/backend_query_test.go index 8c296a6edf72..356ee705eda7 100644 --- a/internal/cloud/backend_query_test.go +++ b/internal/cloud/backend_query_test.go @@ -5,6 +5,10 @@ package cloud import ( "context" + "fmt" + "net/http" + "net/http/httptest" + "os" "path/filepath" "strings" "testing" @@ -12,6 +16,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/cli" + tfe "github.com/hashicorp/go-tfe" + svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform-svchost/auth" + "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/command/arguments" @@ -19,9 +27,12 @@ import ( "github.com/hashicorp/terraform/internal/command/jsonformat" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/depsfile" + "github.com/hashicorp/terraform/internal/httpclient" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terminal" tftesting "github.com/hashicorp/terraform/internal/terraform/testing" + "github.com/hashicorp/terraform/version" + "github.com/zclconf/go-cty/cty" ) func testOperationQuery(t *testing.T, configDir string) (*backendrun.Operation, func(), func(*testing.T) *terminal.TestOutput) { @@ -235,3 +246,425 @@ func TestCloud_queryGenerateConfigOut(t *testing.T) { }) } } + +// indirectHandler wraps a *func so that the mux-registered closure always +// calls whatever function is currently pointed to. This lets tests register +// the route at server-start time (before they have the MockClient) and then +// swap in the real handler body once the MockClient is available. +type indirectHandler struct { + fn *func(http.ResponseWriter, *http.Request) +} + +func (h *indirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if h.fn != nil && *h.fn != nil { + (*h.fn)(w, r) + } +} + +func TestCloud_queryWithPolicyPaths(t *testing.T) { + // Use an indirect handler so we can wire the MockClient reference + // into the handler body after the test server has already started. + var innerFn func(http.ResponseWriter, *http.Request) + ih := &indirectHandler{fn: &innerFn} + + handlers := map[string]func(http.ResponseWriter, *http.Request){ + "/api/v2/queries": ih.ServeHTTP, + } + + b, mc, bCleanup := testBackendAndMocksWithHandlers(t, handlers) + defer bCleanup() + + // Now that we have mc, wire the real handler. + logFile := "./testdata/query-json-basic/query.log" + innerFn = mockQueryV2Handler(t, mc, logFile) + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + op.PolicyPaths = []string{"./policies/allow.tfpolicy.hcl"} + + mockSROWorkspace(t, b, op.Workspace) + + if b.clientV2 == nil { + t.Fatal("b.clientV2 is nil; v2 path cannot be exercised") + } + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + mc.QueryRuns.Lock() + got := mc.QueryRuns.PolicyPaths + mc.QueryRuns.Unlock() + + if len(got) != 1 || got[0] != "./policies/allow.tfpolicy.hcl" { + t.Errorf("expected PolicyPaths = [\"./policies/allow.tfpolicy.hcl\"], got %v", got) + } +} + +func TestCloud_queryWithoutPolicyPaths(t *testing.T) { + // No v2 POST handler — if the v2 path is taken the test server returns 400 + // (the default catch-all), which would fail the operation. + b, mc, bCleanup := testBackendAndMocksWithName(t) + defer bCleanup() + + stream, closeStream := terminal.StreamsForTesting(t) + defer closeStream(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + // PolicyPaths intentionally not set. + + mockSROWorkspace(t, b, op.Workspace) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + // v1 Create path used: PolicyPaths on mock should be nil, and the v1 + // Create must have been called (evidenced by a non-nil Workspace in the + // captured options). + mc.QueryRuns.Lock() + got := mc.QueryRuns.PolicyPaths + wsNil := mc.QueryRuns.CreateOptions.Workspace == nil + mc.QueryRuns.Unlock() + + if len(got) != 0 { + t.Errorf("expected no PolicyPaths captured, got %v", got) + } + if wsNil { + t.Error("expected v1 Create to be called, but CreateOptions.Workspace is nil") + } +} + +func TestCloud_queryJSONWithPolicySummary(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + stream, close := terminal.StreamsForTesting(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-policy") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + + mockSROWorkspace(t, b, op.Workspace) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + outp := close(t) + gotOut := outp.Stdout() + + if !strings.Contains(gotOut, "Policy results for") { + t.Errorf("expected human-readable policy summary in output; got:\n%s", gotOut) + } + // The raw JSON of the record must NOT be printed verbatim. + if strings.Contains(gotOut, `"policy_query_summary"`) { + t.Errorf("raw policy_query_summary JSON leaked into output; got:\n%s", gotOut) + } + // Assert specific content from the log fixture. + if !strings.Contains(gotOut, "list.concept_pet.pets") { + t.Errorf("expected list block address in output; got:\n%s", gotOut) + } + if !strings.Contains(gotOut, "large-roughy") { + t.Errorf("expected identity in output; got:\n%s", gotOut) + } +} + +func TestCloud_queryJSONWithPolicySummaryMalformed(t *testing.T) { + // Use a temp log file with a malformed policy_query_summary record. + logContent := `{"@level":"info","@message":"Terraform 1.14.0-dev","type":"version","ui":"1.2"}` + "\n" + + `{"type":"policy_query_summary","results":"NOT_AN_ARRAY"}` + "\n" + + tmpDir := t.TempDir() + logPath := filepath.Join(tmpDir, "query.log") + if err := os.WriteFile(logPath, []byte(logContent), 0o600); err != nil { + t.Fatal(err) + } + + b, mc, bCleanup := testBackendAndMocksWithName(t) + defer bCleanup() + + stream, close := terminal.StreamsForTesting(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + mockSROWorkspace(t, b, op.Workspace) + + // Inject the log path directly into the mock so the run reads our temp file. + mc.QueryRuns.Lock() + for k := range mc.QueryRuns.logs { + mc.QueryRuns.logs[k] = logPath + } + mc.QueryRuns.Unlock() + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + // Operation must succeed even with a malformed record (skipped gracefully). + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed (malformed record should be skipped): %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + outp := close(t) + gotOut := outp.Stdout() + if strings.Contains(gotOut, "Policy results for") { + t.Errorf("expected no policy summary output for malformed record; got:\n%s", gotOut) + } +} + +// TestCloud_queryJSONWithoutPolicySummary verifies that a normal query run +// (no policy_query_summary records in the log) produces no policy output. +func TestCloud_queryJSONWithoutPolicySummary(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + stream, close := terminal.StreamsForTesting(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + mockSROWorkspace(t, b, op.Workspace) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + outp := close(t) + gotOut := outp.Stdout() + if strings.Contains(gotOut, "Policy results for") { + t.Errorf("expected no policy summary for a plain query run; got:\n%s", gotOut) + } +} + +// TestCloud_queryWithNilClientV2 verifies that when clientV2 is nil (v2 +// unavailable) the backend falls back to the v1 Create path even when +// PolicyPaths are set, and the operation succeeds. +func TestCloud_queryWithNilClientV2(t *testing.T) { + b, mc, bCleanup := testBackendAndMocksWithName(t) + defer bCleanup() + + stream, closeStream := terminal.StreamsForTesting(t) + defer closeStream(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + // Force the v2 client to nil to exercise the fallback path. + b.clientV2 = nil + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + op.PolicyPaths = []string{"./policies/allow.tfpolicy.hcl"} + mockSROWorkspace(t, b, op.Workspace) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + + // The v1 Create must have been invoked (Workspace non-nil in captured options). + mc.QueryRuns.Lock() + wsNil := mc.QueryRuns.CreateOptions.Workspace == nil + mc.QueryRuns.Unlock() + if wsNil { + t.Error("expected v1 Create to be called when clientV2 == nil, but CreateOptions.Workspace is nil") + } +} + +// TestCloud_queryWithPolicyPaths_Enterprise verifies that the v2 POST is +// routed to the correct URL prefix when the TFE service is advertised at a +// non-default base path (simulating a Terraform Enterprise installation). +func TestCloud_queryWithPolicyPaths_Enterprise(t *testing.T) { + const enterpriseHost = "tfe.example.com" + const enterpriseBasePath = "/tfe/api/v2/" + + // Indirect handler: wired after the server and mock are created. + var innerFn func(http.ResponseWriter, *http.Request) + ih := &indirectHandler{fn: &innerFn} + + // The server handles ALL paths; the mux distinguishes by prefix. + mux := http.NewServeMux() + + // Register the Enterprise-prefixed v2 route for the POST /queries call. + mux.Handle(enterpriseBasePath+"queries", ih) + + // The v1 client uses the Enterprise base path for all service calls. Wire + // up the endpoints Configure() needs: ping, entitlement-set, and the + // catch-all organisations 404. + mux.HandleFunc(enterpriseBasePath+"ping", testDefaultRequestHandlers["/api/v2/ping"]) + mux.HandleFunc(enterpriseBasePath+"organizations/hashicorp/entitlement-set", + testDefaultRequestHandlers["/api/v2/organizations/hashicorp/entitlement-set"]) + mux.HandleFunc(enterpriseBasePath+"organizations/", testDefaultRequestHandlers["/api/v2/organizations/"]) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Logf("unexpected %s request for %q", r.Method, r.URL.String()) + w.WriteHeader(http.StatusBadRequest) + }) + s := httptest.NewServer(mux) + t.Cleanup(s.Close) + + // Build a disco that maps the Enterprise hostname to the test server, + // advertising the service at the non-default base path. + entHost := svchost.Hostname(enterpriseHost) + entCredsSrc := auth.StaticCredentialsSource(map[svchost.Hostname]map[string]interface{}{ + entHost: {"token": testCred}, + }) + d := disco.NewWithCredentialsSource(entCredsSrc) + d.SetUserAgent(httpclient.TerraformUserAgent(version.String())) + d.ForceHostServices(entHost, map[string]interface{}{ + "tfe.v2": fmt.Sprintf("%s%s", s.URL, enterpriseBasePath), + }) + + b := New(d) + obj := cty.ObjectVal(map[string]cty.Value{ + "hostname": cty.StringVal(enterpriseHost), + "organization": cty.StringVal("hashicorp"), + "token": cty.NullVal(cty.String), + "workspaces": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal(testBackendSingleWorkspaceName), + "tags": cty.NullVal(cty.Set(cty.String)), + "project": cty.NullVal(cty.String), + }), + }) + + newObj, valDiags := b.PrepareConfig(obj) + if len(valDiags) != 0 { + t.Fatalf("PrepareConfig failed: %s", valDiags.ErrWithWarnings()) + } + confDiags := b.Configure(newObj) + if len(confDiags) != 0 { + t.Fatalf("Configure failed: %s", confDiags.ErrWithWarnings()) + } + + if b.clientV2 == nil { + t.Fatal("b.clientV2 is nil; Enterprise v2 path cannot be exercised") + } + + // Replace the v1 client services with mocks. + mc := NewMockClient() + b.CLI = cli.NewMockUi() + b.client.Applies = mc.Applies + b.client.ConfigurationVersions = mc.ConfigurationVersions + b.client.CostEstimates = mc.CostEstimates + b.client.Organizations = mc.Organizations + b.client.Plans = mc.Plans + b.client.TaskStages = mc.TaskStages + b.client.PolicySetOutcomes = mc.PolicySetOutcomes + b.client.PolicyChecks = mc.PolicyChecks + b.client.QueryRuns = mc.QueryRuns + b.client.Runs = mc.Runs + b.client.RunEvents = mc.RunEvents + b.client.StateVersions = mc.StateVersions + b.client.StateVersionOutputs = mc.StateVersionOutputs + b.client.Variables = mc.Variables + b.client.Workspaces = mc.Workspaces + b.local = testLocalBackend(t, b) + b.input = true + + ctx := context.Background() + if _, err := b.client.Organizations.Create(ctx, tfe.OrganizationCreateOptions{ + Name: tfe.String(b.Organization), + }); err != nil { + t.Fatalf("error creating org: %v", err) + } + if _, err := b.client.Workspaces.Create(ctx, b.Organization, tfe.WorkspaceCreateOptions{ + Name: tfe.String(b.WorkspaceMapping.Name), + }); err != nil { + t.Fatalf("error creating workspace: %v", err) + } + + logFile := "./testdata/query-json-basic/query.log" + innerFn = mockQueryV2Handler(t, mc, logFile) + + stream, closeStream := terminal.StreamsForTesting(t) + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + op, configCleanup, done := testOperationQuery(t, "./testdata/query-json-basic") + defer configCleanup() + defer done(t) + + op.Workspace = testBackendSingleWorkspaceName + op.PolicyPaths = []string{"./policies/allow.tfpolicy.hcl"} + mockSROWorkspace(t, b, op.Workspace) + + run, err := b.Operation(ctx, op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + <-run.Done() + if run.Result != backendrun.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + closeStream(t) + + mc.QueryRuns.Lock() + got := mc.QueryRuns.PolicyPaths + mc.QueryRuns.Unlock() + + if len(got) != 1 || got[0] != "./policies/allow.tfpolicy.hcl" { + t.Errorf("expected PolicyPaths = [\"./policies/allow.tfpolicy.hcl\"], got %v", got) + } +} diff --git a/internal/cloud/testdata/query-json-policy/main.tf b/internal/cloud/testdata/query-json-policy/main.tf new file mode 100644 index 000000000000..81e26bcb5c8b --- /dev/null +++ b/internal/cloud/testdata/query-json-policy/main.tf @@ -0,0 +1,3 @@ +# This directory is used solely to test policy_query_summary log rendering. +# The query configuration is driven by main.tfquery.hcl; there are no +# Terraform-managed resources in this fixture. diff --git a/internal/cloud/testdata/query-json-policy/main.tfquery.hcl b/internal/cloud/testdata/query-json-policy/main.tfquery.hcl new file mode 100644 index 000000000000..a3276bd012a1 --- /dev/null +++ b/internal/cloud/testdata/query-json-policy/main.tfquery.hcl @@ -0,0 +1,3 @@ +list "concept_pet" "pets" { + provider = concept +} diff --git a/internal/cloud/testdata/query-json-policy/query.log b/internal/cloud/testdata/query-json-policy/query.log new file mode 100644 index 000000000000..914cc407b0d4 --- /dev/null +++ b/internal/cloud/testdata/query-json-policy/query.log @@ -0,0 +1,5 @@ +{"@level":"info","@message":"Terraform 1.14.0-dev","@module":"terraform.ui","@timestamp":"2025-08-29T14:07:26.859531+02:00","terraform":"1.14.0-dev","type":"version","ui":"1.2"} +{"@level":"info","@message":"list.concept_pet.pets: Starting query...","@module":"terraform.ui","@timestamp":"2025-08-29T14:07:26.954999+02:00","list_start":{"address":"list.concept_pet.pets","resource_type":"concept_pet"},"type":"list_start"} +{"@level":"info","@message":"list.concept_pet.pets: Result found","@module":"terraform.ui","@timestamp":"2025-08-29T14:07:26.955489+02:00","list_resource_found":{"address":"list.concept_pet.pets","display_name":"This is a large-roughy","identity":{"id":"large-roughy","legs":2},"resource_type":"concept_pet"},"type":"list_resource_found"} +{"@level":"info","@message":"list.concept_pet.pets: List complete","@module":"terraform.ui","@timestamp":"2025-08-29T14:07:26.955538+02:00","list_complete":{"address":"list.concept_pet.pets","resource_type":"concept_pet","total":1},"type":"list_complete"} +{"@level":"info","@message":"Policy results","@module":"terraform.ui","@timestamp":"2025-08-29T14:07:27.000000+02:00","type":"policy_query_summary","list_block_address":"list.concept_pet.pets","overall_result":"pass","results":[{"target_address":"list.concept_pet.pets[0]","result":"pass","identity":{"id":"large-roughy"},"policies":[{"policy_metadata":{"policy_name":"policy.allow","enforcement_level":"mandatory"},"result":"pass","diagnostics":[]}]}],"evaluated_policies":[{"policy_name":"policy.allow","enforcement_level":"mandatory"}]} diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index 1f17f04d63d6..7b736cbe7a95 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -155,6 +155,11 @@ func testBackendNoOperations(t *testing.T) (*Cloud, func()) { } func testBackendWithHandlers(t *testing.T, handlers map[string]func(http.ResponseWriter, *http.Request)) (*Cloud, func()) { + b, _, c := testBackendAndMocksWithHandlers(t, handlers) + return b, c +} + +func testBackendAndMocksWithHandlers(t *testing.T, handlers map[string]func(http.ResponseWriter, *http.Request)) (*Cloud, *MockClient, func()) { obj := cty.ObjectVal(map[string]cty.Value{ "hostname": cty.NullVal(cty.String), "organization": cty.StringVal("hashicorp"), @@ -165,8 +170,7 @@ func testBackendWithHandlers(t *testing.T, handlers map[string]func(http.Respons "project": cty.NullVal(cty.String), }), }) - b, _, c := testBackend(t, obj, handlers) - return b, c + return testBackend(t, obj, handlers) } func testCloudState(t *testing.T) *State { @@ -636,3 +640,71 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]argumen } return vars } + +// mockQueryV2Handler returns an HTTP handler for POST /api/v2/queries. +// It captures decoded policy-paths into mc, inserts a matching QueryRun so +// that subsequent Read/Logs calls work, and returns a minimal JSON:API 201. +func mockQueryV2Handler(t *testing.T, mc *MockClient, logFile string) func(http.ResponseWriter, *http.Request) { + t.Helper() + return func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + + // The Kiota serializer wraps the body in a "data" envelope. + var body struct { + Data struct { + Attributes struct { + PolicyPaths []string `json:"policy-paths"` + } `json:"attributes"` + Relationships struct { + Workspace struct { + Data struct { + ID string `json:"id"` + } `json:"data"` + } `json:"workspace"` + } `json:"relationships"` + } `json:"data"` + } + rawBody, readErr := io.ReadAll(r.Body) + if readErr != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + if err := json.Unmarshal(rawBody, &body); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + + // A missing or empty policy-paths list means the Kiota serializer + // changed the key or envelope shape. Fail fast so the test catches it + // rather than recording a spurious nil slice and passing vacuously. + if len(body.Data.Attributes.PolicyPaths) == 0 { + http.Error(w, "policy-paths is empty or missing in request body", http.StatusBadRequest) + return + } + + mc.QueryRuns.Lock() + mc.QueryRuns.PolicyPaths = body.Data.Attributes.PolicyPaths + + // Synthesise a QueryRun so that Read + Logs work normally. + id := GenerateID("qry-") + logURL := fmt.Sprintf("https://app.terraform.io/_archivist/%s", id) + run := &tfe.QueryRun{ + ID: id, + LogReadURL: logURL, + Status: tfe.QueryRunPending, + } + workspaceID := body.Data.Relationships.Workspace.Data.ID + mc.QueryRuns.logs[logURL] = logFile + mc.QueryRuns.Runs[run.ID] = run + mc.QueryRuns.workspaces[workspaceID] = append(mc.QueryRuns.workspaces[workspaceID], run) + mc.QueryRuns.Unlock() + + w.Header().Set("Content-Type", "application/vnd.api+json") + w.WriteHeader(http.StatusCreated) + // Wrap in a "data" object matching the QueriesEnvelope the SDK deserializes. + fmt.Fprintf(w, `{"data":{"id":%q,"type":"queries","attributes":{"status":"pending","log-read-url":%q}}}`, id, logURL) + } +} diff --git a/internal/cloud/tfe_client_mock.go b/internal/cloud/tfe_client_mock.go index 53e3734aeee0..99c964fe412a 100644 --- a/internal/cloud/tfe_client_mock.go +++ b/internal/cloud/tfe_client_mock.go @@ -2436,6 +2436,10 @@ type MockQueryRuns struct { Runs map[string]*tfe.QueryRun workspaces map[string][]*tfe.QueryRun CreateOptions tfe.QueryRunCreateOptions // captured on each Create call + + // PolicyPaths captures the policy-paths forwarded via the v2 HTTP path. + // It is set by mockQueryV2PostHandler in tests that exercise CLOUD-1. + PolicyPaths []string } func newMockQueryRuns(client *MockClient) *MockQueryRuns { diff --git a/internal/command/views/json_view.go b/internal/command/views/json_view.go index 3fb1dfdcb09a..17adea2cb678 100644 --- a/internal/command/views/json_view.go +++ b/internal/command/views/json_view.go @@ -186,7 +186,7 @@ func (v *JSONView) logPolicyQuerySummary(summary queryPolicySummary) { "list_block_address", summary.ListBlockAddress, "overall_result", summary.OverallResult, "results", summary.Results, - "passed_policies", summary.PassedPolicies, + "evaluated_policies", summary.EvaluatedPolicies, ) } diff --git a/internal/command/views/query_operation.go b/internal/command/views/query_operation.go index ce50c73d93a6..f1b53de43eb4 100644 --- a/internal/command/views/query_operation.go +++ b/internal/command/views/query_operation.go @@ -99,10 +99,18 @@ func (v *QueryOperationHuman) PlanNextStep(planPath string, genConfigPath string } func (v *QueryOperationHuman) Diagnostics(diags tfdiags.Diagnostics) { + remaining := make(tfdiags.Diagnostics, 0, len(diags)) + for _, d := range diags { + extra := tfdiags.ExtraInfo[*tfdiags.ListBlockAddrExtra](d) + if d.Severity() == tfdiags.Warning && extra != nil && extra.ListBlockAddr != "" { + continue // will be emitted by Flush via the warn callback + } + remaining = append(remaining, d) + } if v.queryPolicy != nil { v.queryPolicy.AddWarningDiags(diags) } - v.view.Diagnostics(diags) + v.view.Diagnostics(remaining) } func (v *QueryOperationHuman) PolicyDiagnostics(diags policy.Diagnostics) { @@ -161,10 +169,18 @@ func (v *QueryOperationJSON) PlanNextStep(planPath string, genConfigPath string) } func (v *QueryOperationJSON) Diagnostics(diags tfdiags.Diagnostics) { + remaining := make(tfdiags.Diagnostics, 0, len(diags)) + for _, d := range diags { + extra := tfdiags.ExtraInfo[*tfdiags.ListBlockAddrExtra](d) + if d.Severity() == tfdiags.Warning && extra != nil && extra.ListBlockAddr != "" { + continue // will be emitted by Flush via the warn callback + } + remaining = append(remaining, d) + } if v.queryPolicy != nil { v.queryPolicy.AddWarningDiags(diags) } - v.view.Diagnostics(diags) + v.view.Diagnostics(remaining) } func (v *QueryOperationJSON) PolicyDiagnostics(diags policy.Diagnostics) { diff --git a/internal/command/views/query_policy.go b/internal/command/views/query_policy.go index 2aaa9fa08cb5..53d4738500f5 100644 --- a/internal/command/views/query_policy.go +++ b/internal/command/views/query_policy.go @@ -4,6 +4,7 @@ package views import ( + "encoding/json" "fmt" "maps" "sort" @@ -15,6 +16,17 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) +// RenderPolicyQuerySummaryFromJSON parses a raw policy_query_summary JSON log +// line (as emitted by the remote runner) and renders it as a human-readable +// string. Returns an empty string and a non-nil error on malformed input. +func RenderPolicyQuerySummaryFromJSON(line []byte) (string, error) { + var s queryPolicySummary + if err := json.Unmarshal(line, &s); err != nil { + return "", err + } + return renderQueryPolicySummaryHuman(s), nil +} + type queryPolicyResult string const ( @@ -25,10 +37,10 @@ const ( ) type queryPolicySummary struct { - ListBlockAddress string `json:"list_block_address"` - OverallResult queryPolicyResult `json:"overall_result"` - Results []queryPolicyIdentityResult `json:"results"` - PassedPolicies []viewjson.PolicyMetadata `json:"passed_policies"` + ListBlockAddress string `json:"list_block_address"` + OverallResult queryPolicyResult `json:"overall_result"` + Results []queryPolicyIdentityResult `json:"results"` + EvaluatedPolicies []viewjson.PolicyMetadata `json:"evaluated_policies"` } type queryPolicyIdentityResult struct { @@ -214,23 +226,21 @@ func (b *queryPolicyBlock) summary() queryPolicySummary { return results[i].TargetAddress < results[j].TargetAddress }) - // passed_policies contains all policies evaluated against any identity in - // this block, regardless of whether they passed or failed. The field name - // is intentionally kept as "passed_policies" to match the RFC wire contract - // (the UI uses it for column-header counts, not to filter by pass status). - passedPolicies := make([]viewjson.PolicyMetadata, 0, len(b.PolicyMetadata)) + // evaluated_policies contains all distinct policies evaluated against any + // identity in this block, regardless of whether they passed or failed. + evaluatedPolicies := make([]viewjson.PolicyMetadata, 0, len(b.PolicyMetadata)) for _, metadata := range b.PolicyMetadata { - passedPolicies = append(passedPolicies, metadata) + evaluatedPolicies = append(evaluatedPolicies, metadata) } - sort.Slice(passedPolicies, func(i, j int) bool { - return passedPolicies[i].PolicyName < passedPolicies[j].PolicyName + sort.Slice(evaluatedPolicies, func(i, j int) bool { + return evaluatedPolicies[i].PolicyName < evaluatedPolicies[j].PolicyName }) return queryPolicySummary{ - ListBlockAddress: b.ListBlockAddress, - OverallResult: queryPolicySummaryOverall(results), - Results: results, - PassedPolicies: passedPolicies, + ListBlockAddress: b.ListBlockAddress, + OverallResult: queryPolicySummaryOverall(results), + Results: results, + EvaluatedPolicies: evaluatedPolicies, } } @@ -267,9 +277,18 @@ func queryPolicyResultFromEvaluation(result policy.EvaluateResult) queryPolicyRe func renderQueryPolicySummaryHuman(summary queryPolicySummary) string { var buf strings.Builder - fmt.Fprintf(&buf, "Policy results for %s (%s)\n", summary.ListBlockAddress, strings.ToUpper(string(summary.OverallResult))) + fmt.Fprintf(&buf, "Policy results for %s - %s\n", summary.ListBlockAddress, toPolicyResultLabel(summary.OverallResult)) + + maxLen := 0 + for _, r := range summary.Results { + if l := len(formatQueryPolicyIdentity(r.Identity)); l > maxLen { + maxLen = l + } + } + for _, result := range summary.Results { - fmt.Fprintf(&buf, " %s: %s\n", formatQueryPolicyIdentity(result.Identity), strings.ToUpper(string(result.Result))) + identity := formatQueryPolicyIdentity(result.Identity) + fmt.Fprintf(&buf, " %-*s %s\n", maxLen, identity, toPolicyResultLabel(result.Result)) for _, pol := range result.Policies { if pol.Result != queryPolicyResultFail { continue @@ -283,6 +302,19 @@ func renderQueryPolicySummaryHuman(summary queryPolicySummary) string { return strings.TrimRight(buf.String(), "\n") } +func toPolicyResultLabel(r queryPolicyResult) string { + switch r { + case queryPolicyResultPass: + return "Passed" + case queryPolicyResultFail: + return "Failed" + case queryPolicyResultError: + return "Error" + default: + return "Unknown" + } +} + func formatQueryPolicyIdentity(identity map[string]string) string { if len(identity) == 0 { return "" diff --git a/internal/command/views/query_policy_test.go b/internal/command/views/query_policy_test.go index 179b97958176..cebef6b98879 100644 --- a/internal/command/views/query_policy_test.go +++ b/internal/command/views/query_policy_test.go @@ -174,9 +174,9 @@ func TestQueryOperationJSON_policySummary(t *testing.T) { if len(results) != want.wantResults { t.Fatalf("record[%d] results length = %d, want %d", i, len(results), want.wantResults) } - passedPolicies := got["passed_policies"].([]any) - if len(passedPolicies) != want.wantPolicies { - t.Fatalf("record[%d] passed_policies length = %d, want %d", i, len(passedPolicies), want.wantPolicies) + evaluatedPolicies := got["evaluated_policies"].([]any) + if len(evaluatedPolicies) != want.wantPolicies { + t.Fatalf("record[%d] evaluated_policies length = %d, want %d", i, len(evaluatedPolicies), want.wantPolicies) } for _, rawResult := range results { result := rawResult.(map[string]any) @@ -205,12 +205,13 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { listBlockAddr := "aws_instance.example" tests := []struct { - name string - plan *plans.Plan - schemas *terraform.Schemas - setup func(v *QueryOperationHuman) - want []string - notWant []string + name string + plan *plans.Plan + schemas *terraform.Schemas + setup func(v *QueryOperationHuman) + want []string + notWant []string + countOnce []string // strings that must appear exactly once }{ { name: "mixed_pass_and_fail", @@ -221,12 +222,15 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { }, want: []string{ "Evaluated 2 policies.", - "Policy results for aws_instance.example (FAIL)", - "account=123, id=i-1: PASS", - "account=123, id=i-2: FAIL", + "Policy results for aws_instance.example - Failed", + "account=123, id=i-1", + "Passed", + "account=123, id=i-2", + "Failed", "policy.deny: denied summary (mandatory)", "Policy evaluation skipped", }, + countOnce: []string{"Policy evaluation skipped"}, }, { name: "all_pass", @@ -236,9 +240,10 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { }, want: []string{ "Evaluated 1 policies.", - "Policy results for aws_instance.example (PASS)", - "id=i-1: PASS", - "id=i-2: PASS", + "Policy results for aws_instance.example - Passed", + "id=i-1", + "id=i-2", + "Passed", }, }, { @@ -249,9 +254,10 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { }, want: []string{ "Evaluated 1 policies.", - "Policy results for aws_instance.example (FAIL)", - "id=i-1: FAIL", - "id=i-2: FAIL", + "Policy results for aws_instance.example - Failed", + "id=i-1", + "id=i-2", + "Failed", "policy.deny: denied (mandatory)", }, }, @@ -262,8 +268,9 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { }, want: []string{ "Evaluated 1 policies.", - "Policy results for aws_instance.example (UNKNOWN)", - "id=i-1: UNKNOWN", + "Policy results for aws_instance.example - Unknown", + "id=i-1", + "Unknown", }, }, { @@ -290,6 +297,18 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { "Policy results for", }, }, + { + name: "all_pass_with_count", + setup: func(v *QueryOperationHuman) { + v.PolicyResult("aws_instance.example_0", queryEvalResp(listBlockAddr, map[string]string{"id": "i-1"}, policy.AllowResult, []policyResultSpec{{address: "policy.allow", result: policy.AllowResult}})) + }, + want: []string{ + "Evaluated 1 policies.", + "Policy results for aws_instance.example - Passed", + "id=i-1", + "Passed", + }, + }, } for _, tc := range tests { @@ -310,6 +329,11 @@ func TestQueryOperationHuman_policySummary(t *testing.T) { t.Errorf("expected output NOT to contain %q\nfull output:\n%s", notWant, output) } } + for _, once := range tc.countOnce { + if n := strings.Count(output, once); n != 1 { + t.Errorf("expected %q to appear exactly once, got %d times\nfull output:\n%s", once, n, output) + } + } }) } } @@ -469,9 +493,9 @@ func TestNewQueryJSON_hooksRouteToOperation(t *testing.T) { if len(results) != 1 { t.Fatalf("results length = %d, want 1", len(results)) } - passedPolicies := rec["passed_policies"].([]any) - if len(passedPolicies) != 1 { - t.Fatalf("passed_policies length = %d, want 1", len(passedPolicies)) + evaluatedPolicies := rec["evaluated_policies"].([]any) + if len(evaluatedPolicies) != 1 { + t.Fatalf("evaluated_policies length = %d, want 1", len(evaluatedPolicies)) } } @@ -501,7 +525,7 @@ func TestNewQueryHuman_hooksRouteToOperation(t *testing.T) { q.Operation().Plan(nil, nil) output := done(t).All() - if !strings.Contains(output, "Policy results for aws_instance.example (FAIL)") { + if !strings.Contains(output, "Policy results for aws_instance.example - Failed") { t.Errorf("expected human summary in output\nfull output:\n%s", output) } // The raw "Policy Result" log line must not appear (that would be the base UiHook path). @@ -851,7 +875,7 @@ func TestQueryOperationJSON_recordShape(t *testing.T) { } // Top-level field presence and values. - requiredFields := []string{"@level", "@policy", "type", "list_block_address", "overall_result", "results", "passed_policies"} + requiredFields := []string{"@level", "@policy", "type", "list_block_address", "overall_result", "results", "evaluated_policies"} for _, f := range requiredFields { if _, ok := rec[f]; !ok { t.Errorf("missing required field %q in policy_query_summary record", f) @@ -907,20 +931,20 @@ func TestQueryOperationJSON_recordShape(t *testing.T) { } } - // passed_policies[] shape: 2 distinct policy addresses. - passedPolicies := rec["passed_policies"].([]any) - if len(passedPolicies) != 2 { - t.Fatalf("passed_policies length = %d, want 2", len(passedPolicies)) + // evaluated_policies[] shape: 2 distinct policy addresses. + evaluatedPolicies := rec["evaluated_policies"].([]any) + if len(evaluatedPolicies) != 2 { + t.Fatalf("evaluated_policies length = %d, want 2", len(evaluatedPolicies)) } - for pidx, rawMeta := range passedPolicies { + for pidx, rawMeta := range evaluatedPolicies { if _, ok := rawMeta.(map[string]any); !ok { - t.Errorf("passed_policies[%d] is not an object", pidx) + t.Errorf("evaluated_policies[%d] is not an object", pidx) } } } // TestQueryPolicyView_sortedOutput verifies that both results[] and -// passed_policies[] are emitted in deterministic sorted order. +// evaluated_policies[] are emitted in deterministic sorted order. func TestQueryPolicyView_sortedOutput(t *testing.T) { v := newQueryPolicyView() // Add targets in reverse alphabetical order. @@ -939,9 +963,9 @@ func TestQueryPolicyView_sortedOutput(t *testing.T) { t.Errorf("results not sorted: got [%s, %s], want [a_target, z_target]", s.Results[0].TargetAddress, s.Results[1].TargetAddress) } - if s.PassedPolicies[0].PolicyName != "p.a" || s.PassedPolicies[1].PolicyName != "p.z" { - t.Errorf("passed_policies not sorted: got [%s, %s], want [p.a, p.z]", - s.PassedPolicies[0].PolicyName, s.PassedPolicies[1].PolicyName) + if s.EvaluatedPolicies[0].PolicyName != "p.a" || s.EvaluatedPolicies[1].PolicyName != "p.z" { + t.Errorf("evaluated_policies not sorted: got [%s, %s], want [p.a, p.z]", + s.EvaluatedPolicies[0].PolicyName, s.EvaluatedPolicies[1].PolicyName) } } @@ -1025,3 +1049,62 @@ func TestQueryOperation_nilQueryPolicyPolicyResult(t *testing.T) { t.Run(tt.name, tt.test) } } + +func TestRenderPolicyQuerySummaryFromJSON_valid(t *testing.T) { + // Marshal a known queryPolicySummary and round-trip through the exported + // function, asserting the human-readable output contains expected strings. + summary := queryPolicySummary{ + ListBlockAddress: "list.aws_instance.web", + OverallResult: queryPolicyResultFail, + Results: []queryPolicyIdentityResult{ + { + TargetAddress: "aws_instance.web[0]", + Result: queryPolicyResultFail, + Identity: map[string]string{"id": "i-abc"}, + Policies: []queryPolicyEvalResult{ + { + PolicyMetadata: viewjson.PolicyMetadata{PolicyName: "policy.deny", EnforcementLevel: "mandatory"}, + Result: queryPolicyResultFail, + Diagnostics: []viewjson.Diagnostic{ + {Summary: "resource denied"}, + }, + }, + }, + }, + }, + EvaluatedPolicies: []viewjson.PolicyMetadata{ + {PolicyName: "policy.deny", EnforcementLevel: "mandatory"}, + }, + } + + raw, err := json.Marshal(summary) + if err != nil { + t.Fatalf("failed to marshal summary: %s", err) + } + + out, err := RenderPolicyQuerySummaryFromJSON(raw) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + for _, want := range []string{ + "Policy results for list.aws_instance.web - Failed", + "id=i-abc", + "policy.deny", + "resource denied", + } { + if !strings.Contains(out, want) { + t.Errorf("expected output to contain %q\nfull output:\n%s", want, out) + } + } +} + +func TestRenderPolicyQuerySummaryFromJSON_malformed(t *testing.T) { + out, err := RenderPolicyQuerySummaryFromJSON([]byte("not json")) + if err == nil { + t.Fatal("expected error for malformed JSON, got nil") + } + if out != "" { + t.Errorf("expected empty string on error, got %q", out) + } +} From 2241cf9c44a80cca2d257af3db4fe0d3a302bf01 Mon Sep 17 00:00:00 2001 From: Sebastian Rivera Date: Wed, 12 Aug 2026 14:01:56 -0400 Subject: [PATCH 2/2] query: suppress local policy client init for remote cloud backend When --policies is passed against a cloud backend configured for remote execution, the command layer was unconditionally starting the local tfpolicy-plugin binary, surfacing a spurious 'Failed to connect to policy engine' error even though the operation was about to be delegated entirely to the remote runner. Fix 1 (internal/command/query.go): Guard policy client initialisation with '!isRemoteBackend || b.IsLocalOperations()'. The local client is still started for plain local backends and for the tfc-agent (TF_FORCE_LOCAL_BACKEND) path; it is skipped only when the operation will genuinely run remotely. op.PolicyPaths is always forwarded so the cloud backend can pass paths to the QueryRunCreate API. Fix 2 (internal/cloud/backend.go): In Operation(), when the workspace is in local execution mode, op.PolicyPaths is non-empty, and op.PolicyClient is nil (because IsLocalOperations() was false when the command layer ran), emit a 'Policy evaluation skipped' warning via op.View.Diagnostics before delegating to the local backend. Tests: two new cases in internal/cloud/backend_query_test.go verify (a) the warning fires for the local-exec-mode + nil client scenario, and (b) no warning fires when a policy client is already set. --- internal/cloud/backend.go | 20 +++++ internal/cloud/backend_query_test.go | 106 +++++++++++++++++++++++++++ internal/command/query.go | 2 +- 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 6d3470790295..3cede70c79ee 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -931,6 +931,26 @@ func (b *Cloud) Operation(ctx context.Context, op *backendrun.Operation) (*backe // Record that we're forced to run operations locally to allow the // command package UI to operate correctly b.forceLocal = true + + // When the workspace is in local execution mode (not forced via + // TF_FORCE_LOCAL_BACKEND) and the caller supplied --policies paths but + // no policy client was started (because IsLocalOperations() returned + // false before Operation() was called), warn the user that policy + // evaluation will be skipped. + if isLocalExecutionMode(w.ExecutionMode) && len(op.PolicyPaths) > 0 && op.PolicyClient == nil { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Policy evaluation skipped", + "The workspace is configured for local execution mode but the local "+ + "policy engine could not be initialised before the operation was "+ + "dispatched. Policy paths were provided with -policies but no "+ + "policies will be evaluated. Re-run with TF_FORCE_LOCAL_BACKEND=1 "+ + "or switch the workspace to remote execution mode.", + )) + op.View.Diagnostics(diags) + } + return b.local.Operation(ctx, op) } diff --git a/internal/cloud/backend_query_test.go b/internal/cloud/backend_query_test.go index 356ee705eda7..7a62abb5cd33 100644 --- a/internal/cloud/backend_query_test.go +++ b/internal/cloud/backend_query_test.go @@ -28,6 +28,7 @@ import ( "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/httpclient" + "github.com/hashicorp/terraform/internal/policy" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terminal" tftesting "github.com/hashicorp/terraform/internal/terraform/testing" @@ -668,3 +669,108 @@ func TestCloud_queryWithPolicyPaths_Enterprise(t *testing.T) { t.Errorf("expected PolicyPaths = [\"./policies/allow.tfpolicy.hcl\"], got %v", got) } } + +// TestCloud_opQuery_localExecMode_policyPathsWarning verifies that when the +// workspace is in local execution mode, op.PolicyPaths is non-empty, and +// op.PolicyClient is nil, Operation() emits a "Policy evaluation skipped" +// warning via op.View.Diagnostics before delegating to the local backend. +func TestCloud_opQuery_localExecMode_policyPathsWarning(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + // Switch the workspace to local execution mode. + ctx := context.Background() + _, err := b.client.Workspaces.Update(ctx, b.Organization, b.WorkspaceMapping.Name, + tfe.WorkspaceUpdateOptions{ + ExecutionMode: tfe.String("local"), + }) + if err != nil { + t.Fatalf("error updating workspace execution mode: %v", err) + } + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + operationView := views.NewQuery(arguments.ViewHuman, view).Operation() + + _, configLoader, configCleanup := tftesting.MustLoadConfigForTests(t, "./testdata/query", "tests") + defer configCleanup() + + op := &backendrun.Operation{ + ConfigDir: "./testdata/query", + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + Type: backendrun.OperationTypePlan, + View: operationView, + Query: true, + Workspace: testBackendSingleWorkspaceName, + // PolicyPaths set but PolicyClient intentionally nil — simulates the + // command layer skipping client init because IsLocalOperations()==false. + PolicyPaths: []string{"./policies/allow.tfpolicy.hcl"}, + PolicyClient: nil, + } + + run, runErr := b.Operation(ctx, op) + if runErr != nil { + t.Fatalf("Operation() returned error: %v", runErr) + } + <-run.Done() + + output := done(t) + combined := output.All() + if !strings.Contains(combined, "Policy evaluation skipped") { + t.Errorf("expected 'Policy evaluation skipped' warning in output, got:\n%s", combined) + } +} + +// TestCloud_opQuery_localExecMode_policyPathsWithClient verifies that when +// op.PolicyClient is already set (e.g. TF_FORCE_LOCAL_BACKEND / tfc-agent +// scenario), no "Policy evaluation skipped" warning is emitted. +func TestCloud_opQuery_localExecMode_policyPathsWithClient(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + // Switch the workspace to local execution mode. + ctx := context.Background() + _, err := b.client.Workspaces.Update(ctx, b.Organization, b.WorkspaceMapping.Name, + tfe.WorkspaceUpdateOptions{ + ExecutionMode: tfe.String("local"), + }) + if err != nil { + t.Fatalf("error updating workspace execution mode: %v", err) + } + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + operationView := views.NewQuery(arguments.ViewHuman, view).Operation() + + _, configLoader, configCleanup := tftesting.MustLoadConfigForTests(t, "./testdata/query", "tests") + defer configCleanup() + + // A non-nil PolicyClient signals that the command layer already started the + // local policy engine (TF_FORCE_LOCAL_BACKEND path). Use a no-op stub. + stubClient := &policy.MockClient{} + + op := &backendrun.Operation{ + ConfigDir: "./testdata/query", + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + Type: backendrun.OperationTypePlan, + View: operationView, + Query: true, + Workspace: testBackendSingleWorkspaceName, + PolicyPaths: []string{"./policies/allow.tfpolicy.hcl"}, + PolicyClient: stubClient, + } + + run, runErr := b.Operation(ctx, op) + if runErr != nil { + t.Fatalf("Operation() returned error: %v", runErr) + } + <-run.Done() + + output := done(t) + combined := output.All() + if strings.Contains(combined, "Policy evaluation skipped") { + t.Errorf("unexpected 'Policy evaluation skipped' warning when PolicyClient is set; got:\n%s", combined) + } +} diff --git a/internal/command/query.go b/internal/command/query.go index 27364e9c9161..8c6e9eccbde6 100644 --- a/internal/command/query.go +++ b/internal/command/query.go @@ -126,7 +126,7 @@ func (c *QueryCommand) Run(rawArgs []string) int { return 1 } - if len(args.PolicyPaths) > 0 { + if len(args.PolicyPaths) > 0 && (!isRemoteBackend || b.IsLocalOperations()) { client, policyDiags, stopClient := c.PolicyClient(c.CommandContext(), args.PolicyPaths, backendPolicyEntitlement(be)) // if there has been any errors when setting up the policy client, we log them but // we still proceed with the operation, as a failure to set up the policy client