Skip to content

STACKITRCO-187 - Add option iaas API param agent #1113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aeter wants to merge 1 commit into
base: main
Choose a base branch
from
+154 −81
Open

STACKITRCO-187 - Add option iaas API param agent #1113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/data-sources/server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ data "stackit_server" "example" {
### Read-Only

- `affinity_group` (String) The affinity group the server is assigned to.
- `agent` (Attributes) STACKIT Server Agent as setup on the server (see [below for nested schema](#nestedatt--agent))
- `availability_zone` (String) The availability zone of the server.
- `boot_volume` (Attributes) The boot volume for the server (see [below for nested schema](#nestedatt--boot_volume))
- `created_at` (String) Date-time when the server was created
Expand All @@ -48,6 +49,14 @@ data "stackit_server" "example" {
- `updated_at` (String) Date-time when the server was updated
- `user_data` (String) User data that is passed via cloud-init to the server.

<a id="nestedatt--agent"></a>
### Nested Schema for `agent`

Read-Only:

- `provisioned` (Boolean) Whether a STACKIT Server Agent is provisioned at the server


<a id="nestedatt--boot_volume"></a>
### Nested Schema for `boot_volume`

Expand Down
73 changes: 0 additions & 73 deletions docs/ephemeral-resources/access_token.md
View file
Open in desktop

This file was deleted.

9 changes: 9 additions & 0 deletions docs/resources/server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ import {
### Optional

- `affinity_group` (String) The affinity group the server is assigned to.
- `agent` (Attributes) The STACKIT Server Agent configured for the server (see [below for nested schema](#nestedatt--agent))
- `availability_zone` (String) The availability zone of the server.
- `boot_volume` (Attributes) The boot volume for the server (see [below for nested schema](#nestedatt--boot_volume))
- `desired_status` (String) The desired status of the server resource. Possible values are: `active`, `inactive`, `deallocated`.
Expand All @@ -422,6 +423,14 @@ import {
- `server_id` (String) The server ID.
- `updated_at` (String) Date-time when the server was updated

<a id="nestedatt--agent"></a>
### Nested Schema for `agent`

Optional:

- `provisioned` (Boolean) Whether a STACKIT Server Agent should be provisioned at the server


<a id="nestedatt--boot_volume"></a>
### Nested Schema for `boot_volume`

Expand Down
27 changes: 27 additions & 0 deletions stackit/internal/services/iaas/server/datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type DataSourceModel struct {
ServerId types.String `tfsdk:"server_id"`
MachineType types.String `tfsdk:"machine_type"`
Name types.String `tfsdk:"name"`
Agent types.Object `tfsdk:"agent"`
AvailabilityZone types.String `tfsdk:"availability_zone"`
BootVolume types.Object `tfsdk:"boot_volume"`
ImageId types.String `tfsdk:"image_id"`
Expand All @@ -52,6 +53,10 @@ var bootVolumeDataTypes = map[string]attr.Type{
"delete_on_termination": basetypes.BoolType{},
}

var agentDataTypes = map[string]attr.Type{
"provisioned": basetypes.BoolType{},
}

// NewServerDataSource is a helper function to simplify the provider implementation.
func NewServerDataSource() datasource.DataSource {
return &serverDataSource{}
Expand Down Expand Up @@ -123,6 +128,16 @@ func (d *serverDataSource) Schema(_ context.Context, _ datasource.SchemaRequest,
MarkdownDescription: "Name of the type of the machine for the server. Possible values are documented in [Virtual machine flavors](https://docs.stackit.cloud/products/compute-engine/server/basics/machine-types/)",
Computed: true,
},
"agent": schema.SingleNestedAttribute{
Copy link
Member

@rubenhoenle rubenhoenle Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The acceptance tests don't include your new field yet:

https://github.com/stackitcloud/terraform-provider-stackit/blob/44312a8ebc5b9da9af0608d845b2b733ea349851/stackit/internal/services/iaas/testdata/resource-volume-max.tf

https://github.com/stackitcloud/terraform-provider-stackit/blob/44312a8ebc5b9da9af0608d845b2b733ea349851/stackit/internal/services/iaas/iaas_acc_test.go

Could you also adjust them please?

Copy link
Contributor Author

@aeter aeter Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot run the acceptance tests (if I do, it probably costs money). I did not want to add code to files I cannot test.
If this is a strong requirement for this PR, how can I do this?

Copy link
Member

@rubenhoenle rubenhoenle Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is, we won't accept this without properly adjusted Acc tests.

Copy link
Member

@rubenhoenle rubenhoenle Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're working at STACKIT it shouldn't be a problem for you I hope? 😅

Copy link
Contributor Author

@aeter aeter Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you should be using go-vcr or similar for such integration tests - recording/hardcoding API responses. The way they are written right now - it seems like they belong to an internal CI server, maybe even at the repo of a QA team. stackit-cli doesn't have them. The tests themselves and surely are useful - but how can you ask open source contributors to run them? How much does a sample run cost?

Copy link
Contributor Author

@aeter aeter Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you have reached an agreement of a right implementation, I'll update/run the acceptance tests and update the PR.

Description: "STACKIT Server Agent as setup on the server",
Computed: true,
Attributes: map[string]schema.Attribute{
"provisioned": schema.BoolAttribute{
Description: "Whether a STACKIT Server Agent is provisioned at the server",
Computed: true,
Copy link
Member

@rubenhoenle rubenhoenle Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what the API docs say about the "provisioned" server agent field: https://docs.api.stackit.cloud/documentation/iaas/version/v2#tag/Servers/operation/v2CreateServer

When false the agent IS NOT installed. When true the agent IS installed. When its not set the result depend on the used image and its default provisioning setting.

Since this is not trivial to solve, we as the STACKIT Developer Tools team decided that the default value for the "provisioned" field should be false. Could you please adjust your implementation to respect that decision? 😅

Copy link
Contributor Author

@aeter aeter Jan 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure I understand it right:

  • By the previous code, so far, the param always has defaulted to null. (I think) I keep it as it has always been. Like - don't send any agent-related param to the API.
  • Currently (by my code) the param has 3 states: null, false, true. Default remains null.

It seems like you want to be even more explicit - and to always default the param to false, without a null state (thus - to not use any image default agent provisioning) .
OK. I'll update the code (cannot do it today, I also need to re-test it all).

Copy link
Member

@rubenhoenle rubenhoenle Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@btotev-schwarz We decided to use a default value (false) as the Developer Tools team and maintainers of this Terraform provider for the following reason: The API in it's current state is not declarative-friendly.

If one decides to not send a "provisioned" flag for a server POST request, the "provisioned" flag in the response can either be true or false in the response, but you can't know beforehand. This is not at all compatible with declarative tools like Terraform IMO. Making this work in Terraform will always require some kind of workarounds and I'm not willing to accept this.

  • agent provisioning property is read only, so if you change it, the server need to be recreated (RequiresReplace)

Don't mix up read-only fields and fields marked as RequiresReplace. agent.provisioned can't be read-only because the user has to be able to set a value for it in his Terraform configuration. But it has to be marked as RequiresReplace because the server update endpoint doesn't allow updating the "provisioned" flag of an existing server.

Copy link
Contributor Author

@aeter aeter Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, you'll think I'm stubborn to continue the discussion and propose some technical solution. I know my code in the PR needs adjustments. I know the IaaS API itself probably could use a better design for this parameter (to use strings, like "yes", "no", "image-default" instead of a bool). I'll follow whatever you guys decide (product owners, terraform team).

A small question: is the below solution (keeping the current API model) harder to maintain for the team and harder/unintuitive for the users? Sorry from taking your time with it.

 // If the user omits "agent" and the API returns nothing, Terraform sees null in the config and null in the state. No changes are detected.
// If the API returns {"provisioned": true}, UseStateForUnknown() will copy that object into the plan. Terraform will respect the API's default.
    return schema.Schema{
        Attributes: map[string]schema.Attribute{
            "agent": schema.SingleNestedAttribute{
                // allow user to omit it, or API to set it.
                Optional: true,
                Computed: true,
                
                // If user omits it, keep the old value
                PlanModifiers: []planmodifier.Object{
                    objectplanmodifier.UseStateForUnknown(),
                },

                Attributes: map[string]schema.Attribute{
                    "provisioned": schema.BoolAttribute{
                        Optional: true, // Can be set 
                        Computed: true, // But the API might fill it too.
                        
                        // If user changes this value, force recreation (Immutable behavior)
                        PlanModifiers: []planmodifier.Bool{
                            boolplanmodifier.RequiresReplace(),
                        },
                    },
                },
            },
        },
    }

It seems the "provisioned" flag is not returned from the API at all when the agent: provisioned: ... was not set during image creation. The above will work whether nothing is returned or true/false is returned.

[~/stackit-cli] stackit curl -X GET -H "Content-Type: application/json" https://iaas.api.stackit.cloud/v
2/projects/c904f41c-2f8c-4edb-b966-e87d65f10b64/regions/eu01/servers/5c013c9e-edc0-49a0-ba11-3c971f113bd
5
{
  "availabilityZone": "eu01-m",
  "bootVolume": {
    "deleteOnTermination": true,
    "id": "8af61d89-41cc-49cb-946c-bc75e0cb74c6"
  },
  "createdAt": "2026-01-22T10:30:10Z",
  "id": "5c013c9e-edc0-49a0-ba11-3c971f113bd5",
  "labels": {},
  "launchedAt": "2026-01-22T10:31:37Z",
  "machineType": "t1.1",
  "metadata": {},
  "name": "test2",
  "powerStatus": "RUNNING",
  "status": "ACTIVE",
  "updatedAt": "2026-01-22T10:31:37Z",
  "volumes": [
    "8af61d89-41cc-49cb-946c-bc75e0cb74c6"
  ]
}

If you're interested, I can provide further testing with creating a resource with the current code and modifying it (true/false/missing). Sorry for the interruption.

Copy link
Member

@rubenhoenle rubenhoenle Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is UseStateForUnknown() a workaround or officially supported plan modifier for such cases (https://developer.hashicorp.com/terraform/plugin/framework/resources/plan-modification#usestateforunknown)?

Of course UseStateForUnknown()is a officially supported plan modifier, that's not up for discussion. But in this case I consider using it a workaround, because it wouldn't be needed at all with a declarative-friendly API design. We have decided that we as the Developer Tools team don't have the capacity at the moment to handle such API design failures somehow in our products.

Only because an API design works for the portal, that doesn't mean we have to stick with the same behavior in the Developer Tools (Terraform provider, CLI, ...). The portal isn't a declarative IaC tool which keeps a state, the Terraform provider is. This is why I also don't count in the argument that every touchpoint should integrate this feature the same way - that's only possible if the API matches the requirements of these touchpoints. Which isn't the case here.

The question for me is not if we technically can integrate the API behavior 1:1 in the Terraform provider - of course it's possible. The question for me is if it makes sense. And that's still not the case here from my perspective.

Furthermore, I think that this would be the correct way to go in a Terraform configuration. See the example below.

data "stackit_image" "example" {
  project_id = var.project_id
  image_id   = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
}

resource "stackit_server" "boot-from-volume" {
  project_id = var.project_id
  name       = "example-server"
  boot_volume = {
    size        = 64
    source_type = "image"
    source_id   = stackit_image.example.image_id
  }
  agent = {
    # This way you still have the same behavior. In a declarative manner. 
    # This can be included in the examples and users will be happy.

    # option 1
    provisioned = stackit_image.example.agent.provision_by_default
    # option 2
    # provisioned = stackit_image.example.agent.supported
  }
  availability_zone = "eu01-1"
  machine_type      = "g2i.1"
  keypair_name      = "example-keypair"
}

},
},
},
"availability_zone": schema.StringAttribute{
Description: "The availability zone of the server.",
Computed: true,
Expand Down Expand Up @@ -304,6 +319,18 @@ func mapDataSourceFields(ctx context.Context, serverResp *iaas.Server, model *Da
model.BootVolume = types.ObjectNull(bootVolumeDataTypes)
}

if serverResp.Agent != nil {
agent, diags := types.ObjectValue(agentDataTypes, map[string]attr.Value{
"provisioned": types.BoolPointerValue(serverResp.Agent.Provisioned),
})
if diags.HasError() {
return fmt.Errorf("failed to map agent: %w", core.DiagsToError(diags))
}
model.Agent = agent
} else {
model.Agent = types.ObjectNull(agentDataTypes)
}

if serverResp.UserData != nil && len(*serverResp.UserData) > 0 {
model.UserData = types.StringValue(string(*serverResp.UserData))
}
Expand Down
12 changes: 10 additions & 2 deletions stackit/internal/services/iaas/server/datasource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestMapDataSourceFields(t *testing.T) {
ServerId: types.StringValue("sid"),
Name: types.StringNull(),
AvailabilityZone: types.StringNull(),
Agent: types.ObjectNull(agentTypes),
Labels: types.MapNull(types.StringType),
ImageId: types.StringNull(),
NetworkInterfaces: types.ListNull(types.StringType),
Expand Down Expand Up @@ -78,7 +79,10 @@ func TestMapDataSourceFields(t *testing.T) {
NicId: utils.Ptr("nic2"),
},
},
KeypairName: utils.Ptr("keypair_name"),
KeypairName: utils.Ptr("keypair_name"),
Agent: &iaas.ServerAgent{
Provisioned: utils.Ptr(true),
},
AffinityGroup: utils.Ptr("group_id"),
CreatedAt: utils.Ptr(testTimestamp()),
UpdatedAt: utils.Ptr(testTimestamp()),
Expand All @@ -101,7 +105,10 @@ func TestMapDataSourceFields(t *testing.T) {
types.StringValue("nic1"),
types.StringValue("nic2"),
}),
KeypairName: types.StringValue("keypair_name"),
KeypairName: types.StringValue("keypair_name"),
Agent: types.ObjectValueMust(agentTypes, map[string]attr.Value{
"provisioned": types.BoolValue(true),
}),
AffinityGroup: types.StringValue("group_id"),
CreatedAt: types.StringValue(testTimestampValue),
UpdatedAt: types.StringValue(testTimestampValue),
Expand Down Expand Up @@ -132,6 +139,7 @@ func TestMapDataSourceFields(t *testing.T) {
Labels: types.MapValueMust(types.StringType, map[string]attr.Value{}),
ImageId: types.StringNull(),
NetworkInterfaces: types.ListNull(types.StringType),
Agent: types.ObjectNull(agentTypes),
KeypairName: types.StringNull(),
AffinityGroup: types.StringNull(),
UserData: types.StringNull(),
Expand Down
73 changes: 73 additions & 0 deletions stackit/internal/services/iaas/server/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type Model struct {
ServerId types.String `tfsdk:"server_id"`
MachineType types.String `tfsdk:"machine_type"`
Name types.String `tfsdk:"name"`
Agent types.Object `tfsdk:"agent"`
AvailabilityZone types.String `tfsdk:"availability_zone"`
BootVolume types.Object `tfsdk:"boot_volume"`
ImageId types.String `tfsdk:"image_id"`
Expand All @@ -75,6 +76,11 @@ type Model struct {
DesiredStatus types.String `tfsdk:"desired_status"`
}

// Struct corresponding to Model.Agent
type agentModel struct {
Provisioned types.Bool `tfsdk:"provisioned"`
}

// Struct corresponding to Model.BootVolume
type bootVolumeModel struct {
Id types.String `tfsdk:"id"`
Expand All @@ -95,6 +101,11 @@ var bootVolumeTypes = map[string]attr.Type{
"id": basetypes.StringType{},
}

// Types corresponding to agentModel
var agentTypes = map[string]attr.Type{
"provisioned": basetypes.BoolType{},
}

// NewServerResource is a helper function to simplify the provider implementation.
func NewServerResource() resource.Resource {
return &serverResource{}
Expand Down Expand Up @@ -163,6 +174,14 @@ func (r *serverResource) ValidateConfig(ctx context.Context, req resource.Valida
}
}

var agent = &agentModel{}
if !(model.Agent.IsNull() || model.Agent.IsUnknown()) {
diags := model.Agent.As(ctx, agent, basetypes.ObjectAsOptions{})
if diags.HasError() {
return
}
}

if model.NetworkInterfaces.IsNull() || model.NetworkInterfaces.IsUnknown() || len(model.NetworkInterfaces.Elements()) < 1 {
core.LogAndAddWarning(ctx, &resp.Diagnostics, "No network interfaces configured", "You have no network interfaces configured for this server. This will be a problem when you want to (re-)create this server. Please note that modifying the network interfaces for an existing server will result in a replacement of the resource. We will provide a clear migration path soon.")
}
Expand Down Expand Up @@ -273,6 +292,23 @@ func (r *serverResource) Schema(_ context.Context, _ resource.SchemaRequest, res
Optional: true,
Computed: true,
},
"agent": schema.SingleNestedAttribute{
Description: "The STACKIT Server Agent configured for the server",
Optional: true,
PlanModifiers: []planmodifier.Object{
objectplanmodifier.RequiresReplace(),
},
Attributes: map[string]schema.Attribute{
"provisioned": schema.BoolAttribute{
Description: "Whether a STACKIT Server Agent should be provisioned at the server",
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.RequiresReplace(),
},
},
},
},
"boot_volume": schema.SingleNestedAttribute{
Description: "The boot volume for the server",
Optional: true,
Expand Down Expand Up @@ -962,6 +998,26 @@ func mapFields(ctx context.Context, serverResp *iaas.Server, model *Model, regio
model.NetworkInterfaces = types.ListNull(types.StringType)
}

if serverResp.Agent != nil {
// convert agent model
var modelAgent = &agentModel{}
if !(model.Agent.IsNull() || model.Agent.IsUnknown()) {
diags := model.Agent.As(ctx, modelAgent, basetypes.ObjectAsOptions{})
if diags.HasError() {
return fmt.Errorf("failed to map agent: %w", core.DiagsToError(diags))
}
}
agent, diags := types.ObjectValue(agentTypes, map[string]attr.Value{
"provisioned": types.BoolPointerValue(serverResp.Agent.Provisioned),
})
if diags.HasError() {
return fmt.Errorf("failed to map agentModel: %w", core.DiagsToError(diags))
}
model.Agent = agent
} else {
model.Agent = types.ObjectNull(agentTypes)
}

if serverResp.BootVolume != nil {
// convert boot volume model
var bootVolumeModel = &bootVolumeModel{}
Expand Down Expand Up @@ -1030,6 +1086,14 @@ func toCreatePayload(ctx context.Context, model *Model) (*iaas.CreateServerPaylo
}
}

var agent = &agentModel{}
if !(model.Agent.IsNull() || model.Agent.IsUnknown()) {
diags := model.Agent.As(ctx, agent, basetypes.ObjectAsOptions{})
if diags.HasError() {
return nil, fmt.Errorf("convert agent object to struct: %w", core.DiagsToError(diags))
}
}

labels, err := conversion.ToStringInterfaceMap(ctx, model.Labels)
if err != nil {
return nil, fmt.Errorf("converting to Go map: %w", err)
Expand All @@ -1051,6 +1115,14 @@ func toCreatePayload(ctx context.Context, model *Model) (*iaas.CreateServerPaylo
}
}

var agentPayload *iaas.ServerAgent
// it is set and true, adjust payload
if !agent.Provisioned.IsNull() && !agent.Provisioned.IsUnknown() {
agentPayload = &iaas.ServerAgent{
Provisioned: conversion.BoolValueToPointer(agent.Provisioned),
}
}

var userData *[]byte
if !model.UserData.IsNull() && !model.UserData.IsUnknown() {
src := []byte(model.UserData.ValueString())
Expand Down Expand Up @@ -1080,6 +1152,7 @@ func toCreatePayload(ctx context.Context, model *Model) (*iaas.CreateServerPaylo
return &iaas.CreateServerPayload{
AffinityGroup: conversion.StringValueToPointer(model.AffinityGroup),
AvailabilityZone: conversion.StringValueToPointer(model.AvailabilityZone),
Agent: agentPayload,
BootVolume: bootVolumePayload,
ImageId: conversion.StringValueToPointer(model.ImageId),
KeypairName: conversion.StringValueToPointer(model.KeypairName),
Expand Down
Loading
Loading