diff --git a/README.md b/README.md index 9049d39..0be06f4 100644 --- a/README.md +++ b/README.md @@ -92,12 +92,13 @@ Available resource types: #### rpm / deb / alpine-specific Attributes -| Attribute | Default | Description | -|--------------|---------|-------------------------------------------------------------------------------------------------| -| `mirrorlist` | | Extra upstream mirror base URLs. Requests are load-balanced with failover across `base_url` + `mirrorlist`. | +| Attribute | Default | Description | +|-------------------|---------------|-------------------------------------------------------------------------------------------------| +| `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" } ``` diff --git a/examples/resources/artifactapi_remote_rpm/main.tf b/examples/resources/artifactapi_remote_rpm/main.tf index ebba1e8..8a26f4f 100644 --- a/examples/resources/artifactapi_remote_rpm/main.tf +++ b/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 } diff --git a/internal/provider/datasource_remote.go b/internal/provider/datasource_remote.go index f695a68..872a922 100644 --- a/internal/provider/datasource_remote.go +++ b/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), diff --git a/internal/provider/models.go b/internal/provider/models.go index d7ffcaf..cf67a78 100644 --- a/internal/provider/models.go +++ b/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"` diff --git a/internal/provider/resource_remote.go b/internal/provider/resource_remote.go index 877fd85..c03bdb5 100644 --- a/internal/provider/resource_remote.go +++ b/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() { - return - } - if !mirrorlistPackageTypes[r.packageType] { + if !config.Mirrorlist.IsNull() && !config.Mirrorlist.IsUnknown() && !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), diff --git a/internal/provider/resource_remote_test.go b/internal/provider/resource_remote_test.go index 77de3d6..0426b35 100644 --- a/internal/provider/resource_remote_test.go +++ b/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()) + } + }) +}