Merge pull request 'resource_remote: add mirror_strategy attribute (fixes missing API field)' (#19) from benvin/mirror-strategy into main

Reviewed-on: #19

README.md

@@ -92,12 +92,13 @@ Available resource types:
 
 #### rpm / deb / alpine-specific Attributes
 
-| Attribute    | Default | Description                                                                                     |
+| Attribute         | Default       | Description                                                                                     |
-|--------------|---------|-------------------------------------------------------------------------------------------------|
+|-------------------|---------------|-------------------------------------------------------------------------------------------------|
-| `mirrorlist` |         | Extra upstream mirror base URLs. Requests are load-balanced with failover across `base_url` + `mirrorlist`. |
+| `mirrorlist`      |               | Extra upstream mirror base URLs. Requests are load-balanced with failover across `base_url` + `mirrorlist`. |
+| `mirror_strategy` | `round_robin` | Mirror load-balancing strategy across `base_url` + `mirrorlist`. One of `round_robin` or `least_conn`. |
 
 Only valid on the `artifactapi_remote_rpm`, `artifactapi_remote_deb`, and
-`artifactapi_remote_alpine` resources. Setting it on any other remote type is
+`artifactapi_remote_alpine` resources. Setting either on any other remote type is
 rejected at plan time.
 
 ```hcl
@@ -108,6 +109,7 @@ resource "artifactapi_remote_rpm" "epel" {
     "https://mirror-a.example.net/epel/9/Everything/x86_64",
     "https://mirror-b.example.org/epel/9/Everything/x86_64",
   ]
+  mirror_strategy = "least_conn"
 }
 ```
 

examples/resources/artifactapi_remote_rpm/main.tf

@@ -25,6 +25,10 @@ resource "artifactapi_remote_rpm" "almalinux" {
     "https://mirror.realcompute.io/almalinux",
   ]
 
+  # Load-balancing strategy across base_url + mirrorlist:
+  # "round_robin" (default) or "least_conn".
+  mirror_strategy = "least_conn"
+
   immutable_ttl = 0
   mutable_ttl   = 7200
 }

internal/provider/datasource_remote.go

@@ -31,6 +31,7 @@ func (d *remoteDataSource) Schema(_ context.Context, _ datasource.SchemaRequest,
 			"package_type":       schema.StringAttribute{Computed: true},
 			"base_url":           schema.StringAttribute{Computed: true},
 			"mirrorlist":         schema.ListAttribute{Computed: true, ElementType: types.StringType},
+			"mirror_strategy":    schema.StringAttribute{Computed: true},
 			"description":        schema.StringAttribute{Computed: true},
 			"immutable_ttl":      schema.Int64Attribute{Computed: true},
 			"mutable_ttl":        schema.Int64Attribute{Computed: true},
@@ -59,6 +60,7 @@ type remoteDataSourceModel struct {
 	PackageType       types.String `tfsdk:"package_type"`
 	BaseURL           types.String `tfsdk:"base_url"`
 	Mirrorlist        types.List   `tfsdk:"mirrorlist"`
+	MirrorStrategy    types.String `tfsdk:"mirror_strategy"`
 	Description       types.String `tfsdk:"description"`
 	ImmutableTTL      types.Int64  `tfsdk:"immutable_ttl"`
 	MutableTTL        types.Int64  `tfsdk:"mutable_ttl"`
@@ -110,6 +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:    types.StringValue(remote.MirrorStrategy),
 		Description:       types.StringValue(remote.Description),
 		ImmutableTTL:      types.Int64Value(remote.ImmutableTTL),
 		MutableTTL:        types.Int64Value(remote.MutableTTL),

internal/provider/models.go

@@ -6,6 +6,7 @@ type remoteAPI struct {
 	RepoType          string   `json:"repo_type,omitempty"`
 	BaseURL           string   `json:"base_url"`
 	Mirrorlist        []string `json:"mirrorlist,omitempty"`
+	MirrorStrategy    string   `json:"mirror_strategy,omitempty"`
 	Description       string   `json:"description,omitempty"`
 	Username          string   `json:"username,omitempty"`
 	Password          string   `json:"password,omitempty"`

internal/provider/resource_remote.go

@@ -22,9 +22,17 @@ var (
 )
 
 // mirrorlistPackageTypes are the remote package types for which the mirrorlist
-// attribute is valid; the API load-balances base_url + mirrorlist for these.
+// and mirror_strategy attributes are valid; the API load-balances
+// 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}
+
 type remoteResource struct {
 	client      *apiClient
 	packageType string
@@ -34,6 +42,7 @@ type remoteResourceModel struct {
 	Name              types.String `tfsdk:"name"`
 	BaseURL           types.String `tfsdk:"base_url"`
 	Mirrorlist        types.List   `tfsdk:"mirrorlist"`
+	MirrorStrategy    types.String `tfsdk:"mirror_strategy"`
 	Description       types.String `tfsdk:"description"`
 	Username          types.String `tfsdk:"username"`
 	Password          types.String `tfsdk:"password"`
@@ -101,6 +110,10 @@ func (r *remoteResource) Schema(_ context.Context, _ resource.SchemaRequest, res
 			Optional:    true,
 			ElementType: types.StringType,
 		},
+		"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, Computed: true, Default: stringdefault.StaticString(defaultMirrorStrategy),
+		},
 		"description": schema.StringAttribute{
 			Optional: true, Computed: true, Default: stringdefault.StaticString(""),
 		},
@@ -285,16 +298,29 @@ func (r *remoteResource) ValidateConfig(ctx context.Context, req resource.Valida
 	if resp.Diagnostics.HasError() {
 		return
 	}
-	if config.Mirrorlist.IsNull() || config.Mirrorlist.IsUnknown() {
+	if !config.Mirrorlist.IsNull() && !config.Mirrorlist.IsUnknown() && !mirrorlistPackageTypes[r.packageType] {
-		return
-	}
-	if !mirrorlistPackageTypes[r.packageType] {
 		resp.Diagnostics.AddAttributeError(
 			path.Root("mirrorlist"),
 			"mirrorlist not supported for this remote type",
 			fmt.Sprintf("mirrorlist is only valid on remote rpm/deb/apk (alpine) repos, not %q remotes.", r.packageType),
 		)
 	}
+
+	if !config.MirrorStrategy.IsNull() && !config.MirrorStrategy.IsUnknown() {
+		if !mirrorlistPackageTypes[r.packageType] {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("mirror_strategy"),
+				"mirror_strategy not supported for this remote type",
+				fmt.Sprintf("mirror_strategy is only valid on remote rpm/deb/apk (alpine) repos, not %q remotes.", r.packageType),
+			)
+		} else if v := config.MirrorStrategy.ValueString(); !validMirrorStrategies[v] {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("mirror_strategy"),
+				"invalid mirror_strategy",
+				fmt.Sprintf("mirror_strategy must be \"round_robin\" or \"least_conn\", got %q.", v),
+			)
+		}
+	}
 }
 
 func reconcileOptionalLists(prior, current *remoteResourceModel) {
@@ -326,6 +352,7 @@ func (r *remoteResource) modelToAPI(ctx context.Context, m remoteResourceModel)
 		UpstreamResponseHeaderTimeout: m.UpstreamResponseHeaderTimeout.ValueInt64(),
 	}
 	api.Mirrorlist = listToStrings(ctx, m.Mirrorlist)
+	api.MirrorStrategy = m.MirrorStrategy.ValueString()
 	api.Patterns = listToStrings(ctx, m.Patterns)
 	api.Blocklist = listToStrings(ctx, m.Blocklist)
 	api.MutablePatterns = listToStrings(ctx, m.MutablePatterns)
@@ -336,11 +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:    types.StringValue(mirrorStrategyOrDefault(api.MirrorStrategy)),
 		Description:       types.StringValue(api.Description),
 		Username:          types.StringValue(api.Username),
 		Password:          types.StringValue(api.Password),

internal/provider/resource_remote_test.go

@@ -667,3 +667,156 @@ func TestValidateConfig_Mirrorlist(t *testing.T) {
 		}
 	})
 }
+
+func TestModelToAPI_MirrorStrategy(t *testing.T) {
+	ctx := context.Background()
+
+	for _, pkgType := range []string{"rpm", "deb", "alpine"} {
+		t.Run(pkgType, func(t *testing.T) {
+			r := &remoteResource{packageType: pkgType}
+			model := remoteResourceModel{
+				Name:           types.StringValue("strategy-remote"),
+				BaseURL:        types.StringValue("https://primary.example.com"),
+				MirrorStrategy: types.StringValue("least_conn"),
+			}
+			api := r.modelToAPI(ctx, model)
+			if api.MirrorStrategy != "least_conn" {
+				t.Errorf("MirrorStrategy: expected least_conn, got %q", api.MirrorStrategy)
+			}
+		})
+	}
+}
+
+func TestModelToAPI_MirrorStrategyNull(t *testing.T) {
+	ctx := context.Background()
+	r := &remoteResource{packageType: "rpm"}
+	model := remoteResourceModel{
+		Name:           types.StringValue("no-strategy"),
+		BaseURL:        types.StringValue("https://primary.example.com"),
+		MirrorStrategy: types.StringNull(),
+	}
+	api := r.modelToAPI(ctx, model)
+	if api.MirrorStrategy != "" {
+		t.Errorf("MirrorStrategy: expected \"\" (omitted) for null, got %q", api.MirrorStrategy)
+	}
+}
+
+func TestAPIToModel_MirrorStrategy(t *testing.T) {
+	ctx := context.Background()
+	r := &remoteResource{packageType: "rpm"}
+
+	// Round-trip: mirror_strategy survives API -> Model -> API without churn.
+	api := remoteAPI{Name: "rt", PackageType: "rpm", BaseURL: "https://primary.example.com", MirrorStrategy: "least_conn"}
+	model := r.apiToModel(ctx, api)
+	if model.MirrorStrategy.ValueString() != "least_conn" {
+		t.Errorf("MirrorStrategy round-trip: expected least_conn, got %q", model.MirrorStrategy.ValueString())
+	}
+	if got := r.modelToAPI(ctx, model); got.MirrorStrategy != "least_conn" {
+		t.Errorf("MirrorStrategy round-trip back to API: expected least_conn, got %q", got.MirrorStrategy)
+	}
+
+	// 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.ValueString() != "round_robin" {
+		t.Errorf("MirrorStrategy: expected round_robin default for empty input, got %q", empty.MirrorStrategy.ValueString())
+	}
+}
+
+func TestRemoteResource_SchemaHasMirrorStrategy(t *testing.T) {
+	r := &remoteResource{packageType: "rpm"}
+	var resp resource.SchemaResponse
+	r.Schema(context.Background(), resource.SchemaRequest{}, &resp)
+	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
+// attribute null except mirror_strategy, which is set to the given value (or
+// null when value is "").
+func mirrorStrategyConfig(ctx context.Context, t *testing.T, r *remoteResource, value string) tfsdk.Config {
+	t.Helper()
+	var resp resource.SchemaResponse
+	r.Schema(ctx, resource.SchemaRequest{}, &resp)
+	objType := resp.Schema.Type().TerraformType(ctx).(tftypes.Object)
+
+	attrs := make(map[string]tftypes.Value, len(objType.AttributeTypes))
+	for name, at := range objType.AttributeTypes {
+		attrs[name] = tftypes.NewValue(at, nil)
+	}
+	if value != "" {
+		attrs["mirror_strategy"] = tftypes.NewValue(tftypes.String, value)
+	}
+	return tfsdk.Config{Schema: resp.Schema, Raw: tftypes.NewValue(objType, attrs)}
+}
+
+func TestValidateConfig_MirrorStrategy(t *testing.T) {
+	ctx := context.Background()
+
+	// Supported types accept a valid mirror_strategy.
+	for _, pkgType := range []string{"rpm", "deb", "alpine"} {
+		t.Run("allowed/"+pkgType, func(t *testing.T) {
+			r := &remoteResource{packageType: pkgType}
+			var resp resource.ValidateConfigResponse
+			r.ValidateConfig(ctx, resource.ValidateConfigRequest{Config: mirrorStrategyConfig(ctx, t, r, "least_conn")}, &resp)
+			if resp.Diagnostics.HasError() {
+				t.Errorf("unexpected error for %s: %v", pkgType, resp.Diagnostics.Errors())
+			}
+		})
+	}
+
+	// Unsupported types reject a set mirror_strategy.
+	for _, pkgType := range []string{"docker", "pypi", "helm", "generic"} {
+		t.Run("rejected/"+pkgType, func(t *testing.T) {
+			r := &remoteResource{packageType: pkgType}
+			var resp resource.ValidateConfigResponse
+			r.ValidateConfig(ctx, resource.ValidateConfigRequest{Config: mirrorStrategyConfig(ctx, t, r, "round_robin")}, &resp)
+			if !resp.Diagnostics.HasError() {
+				t.Errorf("expected error setting mirror_strategy on %s remote", pkgType)
+			}
+		})
+	}
+
+	// An invalid enum value is rejected even on a supported type.
+	t.Run("invalid-value-on-rpm", func(t *testing.T) {
+		r := &remoteResource{packageType: "rpm"}
+		var resp resource.ValidateConfigResponse
+		r.ValidateConfig(ctx, resource.ValidateConfigRequest{Config: mirrorStrategyConfig(ctx, t, r, "bogus")}, &resp)
+		if !resp.Diagnostics.HasError() {
+			t.Error("expected error for invalid mirror_strategy value on rpm")
+		}
+	})
+
+	// A null mirror_strategy is fine on any type.
+	t.Run("null-on-docker", func(t *testing.T) {
+		r := &remoteResource{packageType: "docker"}
+		var resp resource.ValidateConfigResponse
+		r.ValidateConfig(ctx, resource.ValidateConfigRequest{Config: mirrorStrategyConfig(ctx, t, r, "")}, &resp)
+		if resp.Diagnostics.HasError() {
+			t.Errorf("unexpected error for null mirror_strategy on docker: %v", resp.Diagnostics.Errors())
+		}
+	})
+}