diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c596192..cc7245a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# v0.23.1 + +FEATURES: +- `meshstack_building_block`: A composition now captures a failed child building block's full run log, using the run-scoped `MESHSTACK_RUN_TOKEN` when meshStack injects it, even if the child has run transparency disabled. + +FIXES: +- `meshstack_building_block_definition`: A definition with an empty `version_spec.inputs = {}` no longer fails with "Provider produced inconsistent result after apply" (null vs empty map). The applied and refreshed values now preserve the configured shape. + # v0.23.0 FEATURES: diff --git a/client/client.go b/client/client.go index b366b318..2cadefcc 100644 --- a/client/client.go +++ b/client/client.go @@ -41,6 +41,10 @@ type Client struct { Workspace MeshWorkspaceClient WorkspaceGroupBinding MeshWorkspaceGroupBindingClient WorkspaceUserBinding MeshWorkspaceUserBindingClient + + // BuildingBlockRunWithRunToken reads run logs authenticated with the privileged run token, letting a + // composition capture a failed child run's system message. Nil unless MESHSTACK_RUN_TOKEN is set. + BuildingBlockRunWithRunToken MeshBuildingBlockRunClient } type Authorization = internal.Authorization @@ -100,6 +104,20 @@ func New(ctx context.Context, rootUrl *url.URL, userAgent string, auth Authoriza }, nil } +// NewBuildingBlockRunClientWithAuth builds a standalone run client with its own authorization, so the +// provider can read run logs with the run token while the main client keeps the workspace credentials. +// It skips the version check the main client already performed. +func NewBuildingBlockRunClientWithAuth(ctx context.Context, rootUrl *url.URL, userAgent string, auth Authorization) MeshBuildingBlockRunClient { + httpClient := internal.WithRetry( + internal.NewHttpClient(rootUrl, userAgent, auth), + internal.RetryOptions{ + MaxRetries: 10, + Backoff: internal.ExponentialBackoff{MinWait: 1 * time.Second, MaxWait: 10 * time.Second}, + }, + ) + return newBuildingBlockRunClient(ctx, httpClient) +} + func checkMeshVersion(ctx context.Context, httpClient internal.HttpClient) error { type MeshInfo struct { Version version.Version `json:"version"` diff --git a/internal/provider/building_block_definition_resource.go b/internal/provider/building_block_definition_resource.go index 611cf1d4..629e5e5e 100644 --- a/internal/provider/building_block_definition_resource.go +++ b/internal/provider/building_block_definition_resource.go @@ -129,6 +129,13 @@ func (r *buildingBlockDefinitionResource) Read(ctx context.Context, req resource Metadata: definitionDto.Metadata, Spec: definitionDto.Spec, } + // Seed the prior inputs shape (null vs empty {}) so SetFromVersionClientDtos preserves it across a + // refresh — the backend collapses the two, so without this an empty-map state would flip to null. + var priorInputs types.Map + resp.Diagnostics.Append(req.State.GetAttribute(ctx, path.Root("version_spec").AtName("inputs"), &priorInputs)...) + if !priorInputs.IsNull() && len(priorInputs.Elements()) == 0 { + state.VersionSpec.Inputs = map[string]*client.MeshBuildingBlockDefinitionInput{} + } versionDtos, err := r.buildingBlockDefinitionVersionClient.List(ctx, bbdUuid) if err != nil { diff --git a/internal/provider/building_block_definition_resource_model.go b/internal/provider/building_block_definition_resource_model.go index c9dd195d..c488fa2b 100644 --- a/internal/provider/building_block_definition_resource_model.go +++ b/internal/provider/building_block_definition_resource_model.go @@ -266,12 +266,15 @@ func (model *buildingBlockDefinition) SetFromVersionClientDtos(diags *diag.Diagn return } latestIndex := len(versionDtos) - 1 - inputsNil := model.VersionSpec.Inputs == nil + // The backend can't distinguish a null inputs map from an empty one (ToClientDto sends {} for both) and + // may return either. Preserve the caller's exact shape (nil vs {}) when the round-trip carries no inputs, + // otherwise Terraform reports a null-vs-empty "inconsistent result after apply". + priorInputs := model.VersionSpec.Inputs model.VersionSpec = buildingBlockDefinitionVersionSpec{ MeshBuildingBlockDefinitionVersionSpec: versionDtos[latestIndex].Spec, } - if inputsNil && len(model.VersionSpec.Inputs) == 0 { - model.VersionSpec.Inputs = nil + if len(priorInputs) == 0 && len(model.VersionSpec.Inputs) == 0 { + model.VersionSpec.Inputs = priorInputs } if !isDraft.IsUnknown() && !isDraft.Get() { diff --git a/internal/provider/building_block_definition_resource_model_test.go b/internal/provider/building_block_definition_resource_model_test.go index 7ad21311..2d1fd6cf 100644 --- a/internal/provider/building_block_definition_resource_model_test.go +++ b/internal/provider/building_block_definition_resource_model_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/meshcloud/terraform-provider-meshstack/client" + "github.com/meshcloud/terraform-provider-meshstack/internal/types/generic" ) var ( @@ -114,3 +115,47 @@ func Test_versionContentHash_ignoresPerVersionFields(t *testing.T) { require.Empty(t, diags) assert.Equal(t, baseHash, mutatedHash, "buildingBlockDefinitionRef/versionNumber/state must not affect the content hash") } + +// The backend collapses a null inputs map and an empty one, so SetFromVersionClientDtos must preserve the +// caller's exact shape when the version carries no inputs — otherwise Terraform reports a null-vs-empty +// "Provider produced inconsistent result after apply". +func Test_SetFromVersionClientDtos_preservesInputsShape(t *testing.T) { + draftState := client.MeshBuildingBlockDefinitionVersionStateDraft.Unwrap() + versionWithNoInputs := client.MeshBuildingBlockDefinitionVersion{ + Metadata: client.MeshBuildingBlockDefinitionVersionMetadata{Uuid: "v1"}, + Spec: client.MeshBuildingBlockDefinitionVersionSpec{ + VersionNumber: new(int64(1)), + State: &draftState, + Implementation: client.MeshBuildingBlockDefinitionImplementation{ + Terraform: &client.MeshBuildingBlockDefinitionTerraformImplementation{}, + }, + // Inputs left nil, mimicking a backend response that omits an empty inputs map. + }, + } + + tests := []struct { + name string + prior map[string]*client.MeshBuildingBlockDefinitionInput + wantNil bool + }{ + {"nil inputs stay nil", nil, true}, + {"empty inputs stay empty (not null)", map[string]*client.MeshBuildingBlockDefinitionInput{}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + model := &buildingBlockDefinition{} + model.VersionSpec.Inputs = tt.prior + + var diags diag.Diagnostics + model.SetFromVersionClientDtos(&diags, generic.KnownValue(true), "bbd", versionWithNoInputs) + require.False(t, diags.HasError(), "unexpected diags: %v", diags.Errors()) + + if tt.wantNil { + require.Nil(t, model.VersionSpec.Inputs) + } else { + require.NotNil(t, model.VersionSpec.Inputs) + require.Empty(t, model.VersionSpec.Inputs) + } + }) + } +} diff --git a/internal/provider/building_block_resource.go b/internal/provider/building_block_resource.go index 179e1074..b1c76db6 100644 --- a/internal/provider/building_block_resource.go +++ b/internal/provider/building_block_resource.go @@ -55,6 +55,10 @@ func NewBuildingBlockResource() resource.Resource { type buildingBlockResource struct { BuildingBlockClient client.MeshBuildingBlockV2Client BuildingBlockRunClient client.MeshBuildingBlockRunClient + // RunTokenRunClient reads run logs with the run token, used for failure diagnostics so a composition + // captures a failed child run's system message regardless of the child's run transparency. Nil unless + // MESHSTACK_RUN_TOKEN is set. + RunTokenRunClient client.MeshBuildingBlockRunClient } func (r *buildingBlockResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { @@ -65,6 +69,7 @@ func (r *buildingBlockResource) Configure(_ context.Context, req resource.Config resp.Diagnostics.Append(configureProviderClient(req.ProviderData, func(client client.Client) { r.BuildingBlockClient = client.BuildingBlockV2 r.BuildingBlockRunClient = client.BuildingBlockRun + r.RunTokenRunClient = client.BuildingBlockRunWithRunToken })...) } @@ -579,7 +584,13 @@ func (r *buildingBlockResource) addRunFailureDiagnostics( if bb == nil || bb.Status == nil || bb.Status.LatestRunUuid == nil { return } - logs, err := r.BuildingBlockRunClient.GetLogs(ctx, *bb.Status.LatestRunUuid) + // Prefer the run-token client: its privileged scope makes the run's system message readable even when + // the (child) building block has run transparency disabled. Falls back to the workspace client. + runClient := r.BuildingBlockRunClient + if r.RunTokenRunClient != nil { + runClient = r.RunTokenRunClient + } + logs, err := runClient.GetLogs(ctx, *bb.Status.LatestRunUuid) if err != nil { // Fetching run logs can legitimately fail: the building block definition may have run // transparency disabled, or the caller's permissions may not allow reading them. Surface it diff --git a/internal/provider/building_block_resource_await_test.go b/internal/provider/building_block_resource_await_test.go index a540f00b..708930df 100644 --- a/internal/provider/building_block_resource_await_test.go +++ b/internal/provider/building_block_resource_await_test.go @@ -175,6 +175,72 @@ func TestAwaitRunWarnsWhenFailedRunLogsUnreadable(t *testing.T) { require.Equal(t, client.BuildingBlockStatusFailed, final.Status.Status) } +// recordingRunLogsClient records whether GetLogs was called and returns a single failed step whose system +// message identifies which client answered, so a test can assert which client was used to fetch logs. +type recordingRunLogsClient struct { + client.MeshBuildingBlockRunClient + systemMessage string + called bool +} + +func (c *recordingRunLogsClient) GetLogs(_ context.Context, _ string) (client.MeshBuildingBlockRunLogs, error) { + c.called = true + return client.MeshBuildingBlockRunLogs{Steps: []client.MeshBuildingBlockRunStepLog{ + {DisplayName: "apply", Status: string(client.BuildingBlockStatusFailed), SystemMessage: new(c.systemMessage)}, + }}, nil +} + +func failedRunStates(runUuid string) []*client.MeshBuildingBlockV2 { + return []*client.MeshBuildingBlockV2{ + bbWithRun(client.BuildingBlockStatusInProgress, runUuid), + bbWithRun(client.BuildingBlockStatusFailed, runUuid), + } +} + +// TestAwaitRunFetchesFailureLogsWithRunTokenClient: when a run-token client is configured, awaitRun reads +// the failed run's logs with it (not the workspace client) so a composition can surface a child's system +// message even with the child's run transparency disabled. +func TestAwaitRunFetchesFailureLogsWithRunTokenClient(t *testing.T) { + t.Parallel() + + stub := &sequencedBBClient{states: failedRunStates("run-broken")} + workspace := &recordingRunLogsClient{systemMessage: "workspace-client logs"} + runToken := &recordingRunLogsClient{systemMessage: "run-token-client logs"} + r := &buildingBlockResource{BuildingBlockClient: stub, BuildingBlockRunClient: workspace, RunTokenRunClient: runToken} + + var diags diag.Diagnostics + r.awaitRun(context.Background(), &diags, "bb-uuid", true, 30*time.Second) + + require.True(t, runToken.called, "the run-token client must fetch failure logs when configured") + require.False(t, workspace.called, "the workspace client must not be used when the run-token client is configured") + require.Contains(t, allErrorDetails(diags), "run-token-client logs") +} + +// TestAwaitRunFetchesFailureLogsWithWorkspaceClientWithoutRunToken: without a run-token client (no +// MESHSTACK_RUN_TOKEN), awaitRun falls back to the workspace client for the failure logs. +func TestAwaitRunFetchesFailureLogsWithWorkspaceClientWithoutRunToken(t *testing.T) { + t.Parallel() + + stub := &sequencedBBClient{states: failedRunStates("run-broken")} + workspace := &recordingRunLogsClient{systemMessage: "workspace-client logs"} + r := &buildingBlockResource{BuildingBlockClient: stub, BuildingBlockRunClient: workspace} + + var diags diag.Diagnostics + r.awaitRun(context.Background(), &diags, "bb-uuid", true, 30*time.Second) + + require.True(t, workspace.called, "the workspace client must fetch failure logs when no run-token client is set") + require.Contains(t, allErrorDetails(diags), "workspace-client logs") +} + +func allErrorDetails(diags diag.Diagnostics) string { + var b strings.Builder + for _, e := range diags.Errors() { + b.WriteString(e.Detail()) + b.WriteByte('\n') + } + return b.String() +} + // TestAwaitRunWarnsOnParkedWaiting: a block parked in WAITING_FOR_*_INPUT cannot proceed from this apply // (a runnable block would already be PENDING). awaitRun returns it as terminal-but-non-fatal with a // waiting-for-input warning, rather than polling to the timeout. diff --git a/internal/provider/provider.go b/internal/provider/provider.go index ddc930e0..75e0c00c 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -73,6 +73,9 @@ const ( envKeyMeshstackApiKey = "MESHSTACK_API_KEY" envKeyMeshstackApiSecret = "MESHSTACK_API_SECRET" envKeyMeshstackApiToken = "MESHSTACK_API_TOKEN" + // envKeyMeshstackRunToken carries the privileged, run-scoped token meshStack injects into a building + // block run. Env-only: it is set per run by the platform, never configured in HCL. + envKeyMeshstackRunToken = "MESHSTACK_RUN_TOKEN" ) func (p *MeshStackProvider) Configure(ctx context.Context, req provider.ConfigureRequest, resp *provider.ConfigureResponse) { @@ -164,6 +167,14 @@ func newProviderClient(ctx context.Context, data MeshStackProviderModel, provide diags.AddError("Failed to create meshStack client.", err.Error()) return } + + // The run token lets a composition read its children's run logs even when the child has run + // transparency disabled. It authorizes only the run-log client; all other calls use the workspace auth. + if runToken := os.Getenv(envKeyMeshstackRunToken); runToken != "" { + providerClient.BuildingBlockRunWithRunToken = client.NewBuildingBlockRunClientWithAuth( + ctx, parsedEndpoint, userAgent, client.NewApiTokenAuthorization(runToken), + ) + } return }