resource_remote: make mirror_strategy Computed to avoid plan churn
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful

The API stores mirror_strategy NOT NULL DEFAULT 'round_robin' and always
returns it on GET, so an Optional-only attribute (mapping empty->null on
read) put 'round_robin' into state while config was null, showing a
perpetual 'round_robin -> null' diff for every remote that did not set it.

- schema: mirror_strategy is now Optional+Computed with a round_robin
  default, mirroring how the description attribute handles an
  API-defaulted scalar; state can hold the API's round_robin with no diff
- apiToModel: pass the API value straight through, settling an empty
  create-response to the round_robin default (no more empty->null)
- datasource: read the value through directly (drop the shared helper)
- tests: assert the GET-returns-round_robin refresh path yields no
  post-apply diff, the empty create-response settles to the default, and
  the schema attribute is Optional+Computed

Removed the now-unused stringOrNull helper.
This commit is contained in:
2026-08-13 19:27:49 +10:00
parent c69c3d9f74
commit 24c11d233a
4 changed files with 44 additions and 16 deletions
+1 -1
View File
@@ -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),
-9
View File
@@ -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)
+16 -2
View File
@@ -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),
+27 -4
View File
@@ -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