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..8e2859f 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: stringOrNull(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 4a1a0f4..bbc1c8b 100644 --- a/internal/provider/helpers.go +++ b/internal/provider/helpers.go @@ -15,6 +15,15 @@ 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/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..221cbde 100644 --- a/internal/provider/resource_remote.go +++ b/internal/provider/resource_remote.go @@ -22,9 +22,13 @@ 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} +// 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 +38,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 +106,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, + }, "description": schema.StringAttribute{ Optional: true, Computed: true, Default: stringdefault.StaticString(""), }, @@ -285,16 +294,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 +348,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) @@ -341,6 +364,7 @@ func (r *remoteResource) apiToModel(ctx context.Context, api remoteAPI) remoteRe Name: types.StringValue(api.Name), BaseURL: types.StringValue(api.BaseURL), Mirrorlist: stringsToList(ctx, api.Mirrorlist), + MirrorStrategy: stringOrNull(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..77d781d 100644 --- a/internal/provider/resource_remote_test.go +++ b/internal/provider/resource_remote_test.go @@ -667,3 +667,133 @@ 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) + } + + // empty mirror_strategy maps to a null string (not "") to avoid perpetual diff. + 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()) + } +} + +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 { + t.Fatal("missing mirror_strategy attribute in schema") + } +} + +// 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()) + } + }) +}