diff --git a/internal/provider/datasource_remote.go b/internal/provider/datasource_remote.go index 8e2859f..872a922 100644 --- a/internal/provider/datasource_remote.go +++ b/internal/provider/datasource_remote.go @@ -112,7 +112,7 @@ func (d *remoteDataSource) Read(ctx context.Context, req datasource.ReadRequest, PackageType: types.StringValue(remote.PackageType), BaseURL: types.StringValue(remote.BaseURL), Mirrorlist: stringsToList(ctx, remote.Mirrorlist), - MirrorStrategy: stringOrNull(remote.MirrorStrategy), + MirrorStrategy: types.StringValue(remote.MirrorStrategy), Description: types.StringValue(remote.Description), ImmutableTTL: types.Int64Value(remote.ImmutableTTL), MutableTTL: types.Int64Value(remote.MutableTTL), diff --git a/internal/provider/helpers.go b/internal/provider/helpers.go index bbc1c8b..4a1a0f4 100644 --- a/internal/provider/helpers.go +++ b/internal/provider/helpers.go @@ -15,15 +15,6 @@ func listToStrings(ctx context.Context, l types.List) []string { return result } -// stringOrNull maps an empty API string to a null attribute so an unset -// optional string does not show a perpetual diff against a null config. -func stringOrNull(s string) types.String { - if s == "" { - return types.StringNull() - } - return types.StringValue(s) -} - func stringsToList(ctx context.Context, ss []string) types.List { if ss == nil { return types.ListNull(types.StringType) diff --git a/internal/provider/resource_remote.go b/internal/provider/resource_remote.go index 221cbde..c03bdb5 100644 --- a/internal/provider/resource_remote.go +++ b/internal/provider/resource_remote.go @@ -26,6 +26,10 @@ var ( // base_url + mirrorlist for these. var mirrorlistPackageTypes = map[string]bool{"rpm": true, "deb": true, "alpine": true} +// defaultMirrorStrategy is the API's server-side default for mirror_strategy +// (stored NOT NULL DEFAULT 'round_robin' and always echoed back on read). +const defaultMirrorStrategy = "round_robin" + // validMirrorStrategies are the accepted mirror_strategy values. var validMirrorStrategies = map[string]bool{"round_robin": true, "least_conn": true} @@ -108,7 +112,7 @@ func (r *remoteResource) Schema(_ context.Context, _ resource.SchemaRequest, res }, "mirror_strategy": schema.StringAttribute{ Description: "Mirror load-balancing strategy across base_url + mirrorlist; only valid on remote rpm/deb/apk (alpine) repos. One of \"round_robin\" (default) or \"least_conn\".", - Optional: true, + Optional: true, Computed: true, Default: stringdefault.StaticString(defaultMirrorStrategy), }, "description": schema.StringAttribute{ Optional: true, Computed: true, Default: stringdefault.StaticString(""), @@ -359,12 +363,22 @@ func (r *remoteResource) modelToAPI(ctx context.Context, m remoteResourceModel) return api } +// mirrorStrategyOrDefault normalizes the API's mirror_strategy to the server +// default when empty (e.g. an older create response), so a Computed+Default +// attribute settles to round_robin instead of an inconsistent-result error. +func mirrorStrategyOrDefault(s string) string { + if s == "" { + return defaultMirrorStrategy + } + return s +} + func (r *remoteResource) apiToModel(ctx context.Context, api remoteAPI) remoteResourceModel { m := remoteResourceModel{ Name: types.StringValue(api.Name), BaseURL: types.StringValue(api.BaseURL), Mirrorlist: stringsToList(ctx, api.Mirrorlist), - MirrorStrategy: stringOrNull(api.MirrorStrategy), + MirrorStrategy: types.StringValue(mirrorStrategyOrDefault(api.MirrorStrategy)), Description: types.StringValue(api.Description), Username: types.StringValue(api.Username), Password: types.StringValue(api.Password), diff --git a/internal/provider/resource_remote_test.go b/internal/provider/resource_remote_test.go index 77d781d..0426b35 100644 --- a/internal/provider/resource_remote_test.go +++ b/internal/provider/resource_remote_test.go @@ -715,10 +715,24 @@ func TestAPIToModel_MirrorStrategy(t *testing.T) { t.Errorf("MirrorStrategy round-trip back to API: expected least_conn, got %q", got.MirrorStrategy) } - // empty mirror_strategy maps to a null string (not "") to avoid perpetual diff. + // The refresh/GET path: the API stores mirror_strategy NOT NULL DEFAULT + // 'round_robin' and always returns it, even when the config omitted it. + // Read must keep round_robin in state (not collapse to null) so that a null + // config — which the Computed+Default schema resolves to round_robin — + // produces no post-apply diff. This is the perpetual-churn regression guard. + getResp := r.apiToModel(ctx, remoteAPI{Name: "g", PackageType: "rpm", BaseURL: "https://x", MirrorStrategy: "round_robin"}) + if getResp.MirrorStrategy.IsNull() { + t.Fatal("MirrorStrategy: GET returning round_robin must not map to null (would churn every plan)") + } + if getResp.MirrorStrategy.ValueString() != "round_robin" { + t.Errorf("MirrorStrategy: expected round_robin from GET, got %q", getResp.MirrorStrategy.ValueString()) + } + + // A create response that omits the field settles to the server default so a + // Computed+Default (round_robin) attribute does not raise inconsistent-result. empty := r.apiToModel(ctx, remoteAPI{Name: "e", PackageType: "rpm", BaseURL: "https://x"}) - if !empty.MirrorStrategy.IsNull() { - t.Errorf("MirrorStrategy: expected null for empty input, got %q", empty.MirrorStrategy.ValueString()) + if empty.MirrorStrategy.ValueString() != "round_robin" { + t.Errorf("MirrorStrategy: expected round_robin default for empty input, got %q", empty.MirrorStrategy.ValueString()) } } @@ -726,9 +740,18 @@ func TestRemoteResource_SchemaHasMirrorStrategy(t *testing.T) { r := &remoteResource{packageType: "rpm"} var resp resource.SchemaResponse r.Schema(context.Background(), resource.SchemaRequest{}, &resp) - if _, ok := resp.Schema.Attributes["mirror_strategy"]; !ok { + attr, ok := resp.Schema.Attributes["mirror_strategy"] + if !ok { t.Fatal("missing mirror_strategy attribute in schema") } + // Must be Computed (with a server-default) so an API-defaulted round_robin + // can live in state while the config is null, without a perpetual diff. + if !attr.IsComputed() { + t.Error("mirror_strategy must be Computed to hold the API's round_robin default without churn") + } + if !attr.IsOptional() { + t.Error("mirror_strategy must remain Optional so users can set least_conn") + } } // mirrorStrategyConfig builds a tfsdk.Config for the remote schema with every