STACKITLB-798 | add ALB TF provider #1198
Conversation
|
imo we should use |
go.mod
Outdated
| github.com/stackitcloud/stackit-sdk-go/services/sqlserverflex v1.3.1 | ||
| github.com/teambition/rrule-go v1.8.2 | ||
| golang.org/x/mod v0.30.0 | ||
| k8s.io/utils v0.0.0-20260108192941-914a6e750570 |
There was a problem hiding this comment.
Please don't add a new module just for a to pointer utils function.
We already have the same function in the core module of our SDK. Replace the ptr.to() to utils.Ptr()
There was a problem hiding this comment.
Ah, thx didn't know this existed
docs/data-sources/alb.md
Outdated
| The example below creates the supporting infrastructure using the STACKIT Terraform provider, including the network, network interface, a public IP address and server resources. | ||
| --- | ||
|
|
||
| # stackit_alb (Data Source) |
There was a problem hiding this comment.
Like @h3adex already suggested, I would prefer as well that the datasource and resource is named application_load_balancer. This makes it easier for first time users to understand for what resource and datasource
There was a problem hiding this comment.
Ya, will change it. I dislike that its so long, but i get why we need it
docs/data-sources/alb.md
Outdated
| ## Setting up supporting infrastructure | ||
|
|
||
|
|
||
| The example below creates the supporting infrastructure using the STACKIT Terraform provider, including the network, network interface, a public IP address and server resources. |
There was a problem hiding this comment.
The description doesn't fit to the datasource
There was a problem hiding this comment.
max acc test is failing
alb_acc_test.go:305: Step 1/4 error: Error running apply: exit status 1
Error: Error updating Application Load Balancer
with stackit_alb.loadbalancer,
on terraform_plugin_test.tf line 169, in resource "stackit_alb" "loadbalancer":
169: resource "stackit_alb" "loadbalancer" {
Calling API for create: 400 Bad Request, status code 400, Body:
{
"code": 3,
"message": "one or more fields are invalid",
"details": [
{
"@type": "type.googleapis.com/google.rpc.BadRequest.FieldViolation",
"field": "loadBalancer.listeners[0].https.certificateConfig.certificateIds[0]",
"description": "The specified certificate \"name-v1-8c81bd317af8a03b8ef0851ccb074eb17d1ad589b540446244a5e593f78ef820\" does not exist."
}
]
}
--- FAIL: TestAccALBResourceMax (117.56s)There was a problem hiding this comment.
Ah, yes thats a resource I wanted to test that not in TF yet. I'll take it out for now
| if providerData.LoadBalancerCustomEndpoint != "" { | ||
| apiClientConfigOptions = append(apiClientConfigOptions, config.WithEndpoint(providerData.LoadBalancerCustomEndpoint)) |
| Description: descriptions["options"], | ||
| Computed: true, | ||
| Attributes: map[string]schema.Attribute{ | ||
| "acl": schema.SetAttribute{ |
There was a problem hiding this comment.
Please be close to the api names. And this is actually an objects which has a property allowed_source_ranges. Keep the struct like in the api, because when the api changes in the future, we can't adjust the provider like the api is adjusted
| "acl": schema.SetAttribute{ | |
| "access_control": schema. SingleNestedAttribute{ |
| resp.Diagnostics.Append(diags...) | ||
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } |
There was a problem hiding this comment.
Add this function here. You may need to update your branch
| } | |
| } | |
| ctx = core.InitProviderContext(ctx) | |
| ) | ||
| resp.State.RemoveResource(ctx) | ||
| return | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| ctx = core.LogResponse(ctx) |
| &resp.Diagnostics, | ||
| err, | ||
| "Reading application load balancer", | ||
| fmt.Sprintf("Load balancer with name %q does not exist in project %q.", name, projectId), |
There was a problem hiding this comment.
| fmt.Sprintf("Load balancer with name %q does not exist in project %q.", name, projectId), | |
| fmt.Sprintf("Application load balancer with name %q does not exist in project %q.", name, projectId), |
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| tflog.Info(ctx, "Load balancer read") |
There was a problem hiding this comment.
| tflog.Info(ctx, "Load balancer read") | |
| tflog.Info(ctx, "Application load balancer read") |
david-mey-STACKIT
force-pushed
the
main
branch
from
7db8e97 to
ac05133
Compare
| protocolOptions := albUtils.ToStringList(albSdk.AllowedListenerProtocolEnumValues) | ||
| roleOptions := albUtils.ToStringList(albSdk.AllowedNetworkRoleEnumValues) | ||
| errorOptions := albUtils.ToStringList(albSdk.AllowedLoadBalancerErrorTypesEnumValues) | ||
| servicePlanOptions := []string{"p10"} |
There was a problem hiding this comment.
same like mentioned for the datasource. check if the sdk has already the enums as list and use it here to decrease maintenance overhead
| func (r *applicationLoadBalancerResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { | ||
| // Do nothing! | ||
| // We do not validate the TF input, because the API does that and | ||
| // maintaining and syncing between them is not worth it, because | ||
| // 400 Bad Request error gives all the details the user needs via the API in TF, which | ||
| // also is the single source of truth. | ||
| } |
There was a problem hiding this comment.
This is an optional method which is not mandatory to implement for the terraform provider. So you can just remove it :)
| Validators: []validator.String{ | ||
| stringvalidator.LengthBetween(4, 6), | ||
| }, |
| "errors": schema.SetNestedAttribute{ | ||
| Description: descriptions["errors"], | ||
| Computed: true, | ||
| PlanModifiers: []planmodifier.Set{ | ||
| setplanmodifier.UseStateForUnknown(), | ||
| }, | ||
| NestedObject: schema.NestedAttributeObject{ | ||
| Attributes: map[string]schema.Attribute{ | ||
| "type": schema.StringAttribute{ | ||
| Description: descriptions["errors.type"], | ||
| Computed: true, | ||
| PlanModifiers: []planmodifier.String{ | ||
| stringplanmodifier.UseStateForUnknown(), | ||
| }, | ||
| }, | ||
| "description": schema.StringAttribute{ | ||
| Description: descriptions["errors.description"], | ||
| Computed: true, | ||
| PlanModifiers: []planmodifier.String{ | ||
| stringplanmodifier.UseStateForUnknown(), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
These attributes shouldn't need the plan modifier UseStateForUnknown
| "observability_metrics": "Observability metrics configuration.", | ||
| "observability_metrics_credentials_ref": "Credentials reference for metrics. This reference is created via the observability create endpoint and the credential needs to contain the basic auth username and password for the metrics solution the push URL points to. Then this enables monitoring via remote write for the Application Load Balancer.", | ||
| "observability_metrics_push_url": "The Observability(Metrics)/Prometheus remote write push URL you want the metrics to be shipped to.", | ||
| "plan_id": "Service Plan configures the size of the Application Load Balancer. " + utils.FormatPossibleValues(servicePlanOptions...) + ". See available plans via STACKIT CLI 'stackit alb plans' or API https://docs.api.stackit.cloud/documentation/alb/version/v2#tag/Project/operation/APIService_ListPlans", |
There was a problem hiding this comment.
Currently we have the alb plans still as a beta command
| "plan_id": "Service Plan configures the size of the Application Load Balancer. " + utils.FormatPossibleValues(servicePlanOptions...) + ". See available plans via STACKIT CLI 'stackit alb plans' or API https://docs.api.stackit.cloud/documentation/alb/version/v2#tag/Project/operation/APIService_ListPlans", | |
| "plan_id": "Service Plan configures the size of the Application Load Balancer. " + utils.FormatPossibleValues(servicePlanOptions...) + ". See available plans via STACKIT CLI 'stackit beta alb plans' or API https://docs.api.stackit.cloud/documentation/alb/version/v2#tag/Project/operation/APIService_ListPlans", |
There was a problem hiding this comment.
Sure, will change. Will you take care of updating this once the CLI is GA? Is updating the CLI complex or is there an automated tool for it?
| target_port = var.target_pool_port | ||
| targets = [ | ||
| { | ||
| display_name = var.target_display_name |
There was a problem hiding this comment.
regarding the docs display_name is optional and therefore not be set in the min test
| resource.TestCheckResourceAttrSet("stackit_application_load_balancer.loadbalancer", "target_security_group.id"), | ||
| resource.TestCheckResourceAttrSet("stackit_application_load_balancer.loadbalancer", "target_security_group.name"), | ||
| resource.TestCheckResourceAttrSet("stackit_application_load_balancer.loadbalancer", "load_balancer_security_group.id"), | ||
| resource.TestCheckResourceAttrSet("stackit_application_load_balancer.loadbalancer", "load_balancer_security_group.name"), |
There was a problem hiding this comment.
Here you check the resource instead of the datasource
| maps.Copy(tempConfig, testConfigVarsMin) | ||
| tempConfig["target_pool_port"] = config.StringVariable("5431") |
There was a problem hiding this comment.
aren't there more fields which can be updated in-place in the minimal config? like rename the target pool or change the target pool at all or change the listeners?
| Check: resource.ComposeAggregateTestCheckFunc( | ||
| resource.TestCheckResourceAttr("stackit_application_load_balancer.loadbalancer", "project_id", testutil.ConvertConfigVariable(testConfigVarsMin["project_id"])), | ||
| resource.TestCheckResourceAttr("stackit_application_load_balancer.loadbalancer", "name", testutil.ConvertConfigVariable(testConfigVarsMin["loadbalancer_name"])), | ||
| resource.TestCheckResourceAttr("stackit_application_load_balancer.loadbalancer", "target_pools.0.target_port", testutil.ConvertConfigVariable(configVarsMinUpdated()["target_pool_port"])), | ||
| ), |
There was a problem hiding this comment.
please check here all fields like in the Create step. With the only difference that here the updated configVars is used
| Check: resource.ComposeAggregateTestCheckFunc( | ||
| resource.TestCheckResourceAttr("stackit_application_load_balancer.loadbalancer", "project_id", testutil.ConvertConfigVariable(testConfigVarsMax["project_id"])), | ||
| resource.TestCheckResourceAttr("stackit_application_load_balancer.loadbalancer", "name", testutil.ConvertConfigVariable(testConfigVarsMax["loadbalancer_name"])), |
There was a problem hiding this comment.
same here. you can basically copy all the checks from the Create step and replace testConfigVarsMax by configVarsMaxUpdated()
3 participants
Description
STACKITLB-798
Add Application Load Balancer provider (API: https://docs.api.stackit.cloud/documentation/alb/version/v2)
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)