Skip to content

Feature/dns skip wait and partial state #1052

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
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Feature/dns skip wait and partial state #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8712b73
add continue attribute for observability service alert config
PatrickKoss Sep 15, 2025
7ef4213
adjust the acceptance test observability
PatrickKoss Sep 15, 2025
702122b
adjust docs
PatrickKoss Sep 15, 2025
56eb265
add continue in another case
PatrickKoss Sep 17, 2025
b405ce7
Merge branch 'main' into main
PatrickKoss Sep 18, 2025
188f0b7
Merge branch 'stackitcloud:main' into main
PatrickKoss Sep 19, 2025
13fdd53
remove continue attribute from root
PatrickKoss Sep 19, 2025
113bbb9
fix acc test
PatrickKoss Sep 19, 2025
e073ec2
Merge branch 'stackitcloud:main' into main
PatrickKoss Sep 19, 2025
8118f17
fix docs
PatrickKoss Sep 19, 2025
3e1a403
fix unit tests
PatrickKoss Sep 19, 2025
039719f
remove route types
PatrickKoss Sep 19, 2025
6ffe516
Merge branch 'main' into main
rubenhoenle Sep 19, 2025
e74f9f8
Merge branch 'stackitcloud:main' into main
PatrickKoss Oct 29, 2025
4e99f0d
Merge branch 'stackitcloud:main' into main
PatrickKoss Nov 3, 2025
55183c5
Merge branch 'stackitcloud:main' into main
PatrickKoss Nov 10, 2025
ee3a0c8
add skip wait and set partial model
PatrickKoss Nov 10, 2025
76fc503
fix linting errors
PatrickKoss Nov 10, 2025
de09817
revert formatting
PatrickKoss Nov 11, 2025
e7649c2
revert formatting
PatrickKoss Nov 11, 2025
037cece
import state
PatrickKoss Nov 11, 2025
265836f
downlint lint from releases + remove read id check
PatrickKoss Nov 12, 2025
ba8ecc8
Merge branch 'main' into feature/dns-skip-wait
PatrickKoss Nov 12, 2025
1196efb
fix pipeline linting
PatrickKoss Nov 12, 2025
50f1f37
adjust SetModelFieldsToNull to handle complex objects and lists
PatrickKoss Nov 13, 2025
873f875
fix linting
PatrickKoss Nov 13, 2025
6e89bf9
fix linting
PatrickKoss Nov 13, 2025
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
13 changes: 11 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
ROOT_DIR ?= $(shell git rev-parse --show-toplevel)
SCRIPTS_BASE ?= $(ROOT_DIR)/scripts
BIN_DIR ?= $(ROOT_DIR)/bin

# https://github.com/golangci/golangci-lint/releases
GOLANGCI_LINT_VERSION = 1.64.8
GOLANGCI_LINT = $(BIN_DIR)/golangci-lint

# SETUP AND TOOL INITIALIZATION TASKS
project-help:
Expand All @@ -8,10 +13,14 @@ project-help:
project-tools:
@$(SCRIPTS_BASE)/project.sh tools

# GOLANGCI-LINT INSTALLATION
$(GOLANGCI_LINT):
@GOLANGCI_LINT_VERSION=$(GOLANGCI_LINT_VERSION) $(SCRIPTS_BASE)/install-golangci-lint.sh

# LINT
lint-golangci-lint:
lint-golangci-lint: $(GOLANGCI_LINT)
@echo "Linting with golangci-lint"
@$(SCRIPTS_BASE)/lint-golangci-lint.sh
@$(SCRIPTS_BASE)/lint-golangci-lint.sh $(GOLANGCI_LINT)

lint-tf:
@echo "Linting terraform files"
Expand Down
42 changes: 42 additions & 0 deletions scripts/install-golangci-lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env bash
set -e
. $(dirname ${0})/utility.sh

BINARY_NAME=golangci-lint
INSTALL_TO=${BIN_DIR}/${BINARY_NAME}

install() {
echo " installing ${BINARY_NAME} ${GOLANGCI_LINT_VERSION}"

TYPE=windows
if [[ "${OSTYPE}" == linux* ]]; then
TYPE=linux
elif [[ "${OSTYPE}" == darwin* ]]; then
TYPE=darwin
fi

case $(uname -m) in
arm64|aarch64)
ARCH=arm64
;;
*)
ARCH=amd64
;;
esac

BASE_URL=https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}
URL=${BASE_URL}/golangci-lint-${GOLANGCI_LINT_VERSION}-${TYPE}-${ARCH}.tar.gz
echo " Downloading: ${URL}"
download ${URL} | tar --extract --gzip --strip-components 1 --preserve-permissions -C ${BIN_DIR} -f-

# Ensure the binary has the correct name
if [ -f "${BIN_DIR}/golangci-lint" ] && [ "${BIN_DIR}/golangci-lint" != "${INSTALL_TO}" ]; then
mv "${BIN_DIR}/golangci-lint" "${INSTALL_TO}"
fi
}

get_version() {
${INSTALL_TO} version 2>/dev/null | awk '{print $4}'
}

update_if_necessary ${GOLANGCI_LINT_VERSION}
13 changes: 7 additions & 6 deletions scripts/lint-golangci-lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
#!/usr/bin/env bash
# This script lints the SDK modules and the internal examples
# Pre-requisites: golangci-lint
# Pre-requisites: golangci-lint (provided by Makefile or system)
set -eo pipefail

ROOT_DIR=$(git rev-parse --show-toplevel)
GOLANG_CI_YAML_PATH="${ROOT_DIR}/golang-ci.yaml"
GOLANG_CI_ARGS="--allow-parallel-runners --timeout=5m --config=${GOLANG_CI_YAML_PATH}"

if type -p golangci-lint >/dev/null; then
:
else
echo "golangci-lint not installed, unable to proceed."
# Use provided golangci-lint binary or fallback to system installation
GOLANGCI_LINT_BIN="${1:-golangci-lint}"

if [ ! -x "${GOLANGCI_LINT_BIN}" ] && ! type -p "${GOLANGCI_LINT_BIN}" >/dev/null; then
echo "golangci-lint not found at ${GOLANGCI_LINT_BIN} and not installed in PATH, unable to proceed."
exit 1
fi

cd ${ROOT_DIR}
golangci-lint run ${GOLANG_CI_ARGS}
${GOLANGCI_LINT_BIN} run ${GOLANG_CI_ARGS}
46 changes: 46 additions & 0 deletions scripts/utility.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/usr/bin/env bash
# Common utility functions for tool installation scripts

ROOT_DIR=$(git rev-parse --show-toplevel)
BIN_DIR="${ROOT_DIR}/bin"

# Ensure bin directory exists
mkdir -p "${BIN_DIR}"

# Download function using curl
download() {
local URL=$1
if command -v curl &> /dev/null; then
curl -sSfL "${URL}"
elif command -v wget &> /dev/null; then
wget -qO- "${URL}"
else
echo "Error: Neither curl nor wget found. Please install one of them."
exit 1
fi
}

# Update tool if necessary
update_if_necessary() {
local EXPECTED_VERSION=$1

if [ -x "${INSTALL_TO}" ]; then
CURRENT_VERSION=$(get_version 2>/dev/null || echo "")
if [ "${CURRENT_VERSION}" = "${EXPECTED_VERSION}" ]; then
echo " ${BINARY_NAME} ${EXPECTED_VERSION} already installed"
return 0
else
echo " ${BINARY_NAME} version mismatch (current: ${CURRENT_VERSION}, expected: ${EXPECTED_VERSION})"
echo " updating to ${EXPECTED_VERSION}..."
fi
fi

install

INSTALLED_VERSION=$(get_version 2>/dev/null || echo "unknown")
if [ "${INSTALLED_VERSION}" = "${EXPECTED_VERSION}" ]; then
echo " ${BINARY_NAME} ${EXPECTED_VERSION} installed successfully"
else
echo " Warning: installed version (${INSTALLED_VERSION}) does not match expected version (${EXPECTED_VERSION})"
fi
}
73 changes: 63 additions & 10 deletions stackit/internal/services/dns/recordset/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package dns

import (
"context"
"errors"
"fmt"
"net/http"
"strings"

"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
Expand All @@ -16,6 +18,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/stackitcloud/stackit-sdk-go/core/oapierror"
"github.com/stackitcloud/stackit-sdk-go/services/dns"
"github.com/stackitcloud/stackit-sdk-go/services/dns/wait"
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion"
Expand Down Expand Up @@ -219,15 +222,27 @@ func (r *recordSetResource) Create(ctx context.Context, req resource.CreateReque
}

// Write id attributes to state before polling via the wait handler - just in case anything goes wrong during the wait handler
utils.SetAndLogStateFields(ctx, &resp.Diagnostics, &resp.State, map[string]any{
"project_id": projectId,
"zone_id": zoneId,
"record_set_id": *recordSetResp.Rrset.Id,
})
recordSetId := *recordSetResp.Rrset.Id
model.RecordSetId = types.StringValue(recordSetId)
model.Id = utils.BuildInternalTerraformId(projectId, zoneId, recordSetId)

// Set all unknown/null fields to null before saving state
if err := utils.SetModelFieldsToNull(ctx, &model); err != nil {
Copy link
Member

@rubenhoenle rubenhoenle Nov 12, 2025

Choose a reason for hiding this comment

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

Sorry, but I just don't get why one would want to have this. What's the point of this?

Copy link
Contributor Author

@PatrickKoss PatrickKoss Nov 12, 2025

Choose a reason for hiding this comment

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

It might be because of weird clients. Currently we only set project_id and zone_id in the state before waiting. This lead to the following error if the waiting is skipped which some clients want:

"error": "cannot get a terraform workspace for resource: cannot ensure tfstate file: cannot check whether the state is empty: cannot work with a non-string id: <nil>", "errorVerbose": "cannot work with a non-string id: <nil>

So I have set the id as well in the helper function. I think I observed an error in the past that the client got an error because some field in the state were "unknown". But I can no longer find this error message anymore. So currently I get:

apply failed: Provider produced inconsistent result after apply: When applying changes to stackit_dns_zone.example-zone, provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced an unexpected new value: .description: was cty.StringVal("Example DNS zone for demonstration"), but now null.

      This is a bug in the provider, which should be reported in the provider's own issue tracker.
      Provider produced inconsistent result after apply: When applying changes to stackit_dns_zone.example-zone, provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced an unexpected new value: .dns_name: was cty.StringVal("patrick.test.patrick.patrick"), but now null.

      This is a bug in the provider, which should be reported in the provider's own issue tracker.
      Provider produced inconsistent result after apply: When applying changes to stackit_dns_zone.example-zone, provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced an unexpected new value: .name: was cty.StringVal("example-zone"), but now null.

      This is a bug in the provider, which should be reported in the provider's own issue tracker.
      Provider produced inconsistent result after apply: When applying changes to stackit_dns_zone.example-zone, provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced an unexpected new value: .is_reverse_zone: was cty.False, but now null.

      This is a bug in the provider, which should be reported in the provider's own issue tracker.
      Provider produced inconsistent result after apply: When applying changes to stackit_dns_zone.example-zone, provider "provider[\"registry.terraform.io/stackitcloud/stackit\"]" produced an unexpected new value: .type: was cty.StringVal("primary"), but now null.

      This is a bug in the provider, which should be reported in the provider's own issue tracker.

Because of it the client wants to destroy the resource:

"error": "cannot run plan: plan failed: Instance cannot be destroyed: Resource stackit_dns_zone.example-zone has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.", "errorVerbose": "plan failed: Instance cannot be destroyed: Resource stackit_dns_zone.example-zone has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.

For some reason if you set the fields to null instead of unknown the client accepts it and proceeds correctly. Maybe we need to take a look together into the topic. If you have some better ways to handle this case feel free to suggest :)

Copy link
Member

@rubenhoenle rubenhoenle Nov 13, 2025

Choose a reason for hiding this comment

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

Just to make sure I don't mess things up here, what do you mean with client?

Copy link
Contributor Author

@PatrickKoss PatrickKoss Nov 13, 2025

Choose a reason for hiding this comment

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

crossplane+upjet that then executes terraform cli commands

Copy link
Member

@rubenhoenle rubenhoenle Nov 13, 2025

Choose a reason for hiding this comment

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

image

Messages like this are there for a reason by Terraform. You would break this behavior with this change.

Copy link
Contributor Author

@PatrickKoss PatrickKoss Nov 13, 2025

Choose a reason for hiding this comment

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

I suspect that is a problem with complex objects, list of lists and list of complex objects in the utils function SetModelFieldsToNull.
I also tried adding the same logic as in zone to iaas network and added alot of unit tests to provoke the error and couldn´t reproduce. You can check it here if you want.

Can you provide the input parameters so I can add unit tests for this case to verify if it happens in the implementation or not?
Additionally you can check with in your setup as well if the added functionality resolves the issue.

Copy link
Contributor Author

@PatrickKoss PatrickKoss Nov 13, 2025

Choose a reason for hiding this comment

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

or do you imply that it is perfectly fine to have errors? because if we want to use upjet to generate a crossplane provider we cannot accept such error since it simply does not work :D

core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating record set", fmt.Sprintf("Setting model fields to null: %v", err))
return
}

diags = resp.State.Set(ctx, model)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

if !utils.ShouldWait() {
tflog.Info(ctx, "Skipping wait; async mode for Crossplane/Upjet")
return
}

waitResp, err := wait.CreateRecordSetWaitHandler(ctx, r.client, projectId, zoneId, *recordSetResp.Rrset.Id).WaitWithContext(ctx)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating record set", fmt.Sprintf("Instance creation waiting: %v", err))
Expand Down Expand Up @@ -266,6 +281,12 @@ func (r *recordSetResource) Read(ctx context.Context, req resource.ReadRequest,

recordSetResp, err := r.client.GetRecordSet(ctx, projectId, zoneId, recordSetId).Execute()
if err != nil {
var oapiErr *oapierror.GenericOpenAPIError
ok := errors.As(err, &oapiErr)
if ok && (oapiErr.StatusCode == http.StatusNotFound || oapiErr.StatusCode == http.StatusGone) {
resp.State.RemoveResource(ctx)
return
}
core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading record set", fmt.Sprintf("Calling API: %v", err))
return
}
Expand Down Expand Up @@ -319,6 +340,12 @@ func (r *recordSetResource) Update(ctx context.Context, req resource.UpdateReque
core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating record set", err.Error())
return
}

if !utils.ShouldWait() {
tflog.Info(ctx, "Skipping wait; async mode for Crossplane/Upjet")
return
}

waitResp, err := wait.PartialUpdateRecordSetWaitHandler(ctx, r.client, projectId, zoneId, recordSetId).WaitWithContext(ctx)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating record set", fmt.Sprintf("Instance update waiting: %v", err))
Expand Down Expand Up @@ -358,8 +385,22 @@ func (r *recordSetResource) Delete(ctx context.Context, req resource.DeleteReque
// Delete existing record set
_, err := r.client.DeleteRecordSet(ctx, projectId, zoneId, recordSetId).Execute()
if err != nil {
// If resource is already gone (404 or 410), treat as success for idempotency
var oapiErr *oapierror.GenericOpenAPIError
ok := errors.As(err, &oapiErr)
if ok && (oapiErr.StatusCode == http.StatusNotFound || oapiErr.StatusCode == http.StatusGone) {
tflog.Info(ctx, "Record set already deleted")
return
}
core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting record set", fmt.Sprintf("Calling API: %v", err))
return
}

if !utils.ShouldWait() {
tflog.Info(ctx, "Skipping wait; async mode for Crossplane/Upjet")
return
}

_, err = wait.DeleteRecordSetWaitHandler(ctx, r.client, projectId, zoneId, recordSetId).WaitWithContext(ctx)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting record set", fmt.Sprintf("Instance deletion waiting: %v", err))
Expand All @@ -380,11 +421,23 @@ func (r *recordSetResource) ImportState(ctx context.Context, req resource.Import
return
}

utils.SetAndLogStateFields(ctx, &resp.Diagnostics, &resp.State, map[string]interface{}{
"project_id": idParts[0],
"zone_id": idParts[1],
"record_set_id": idParts[2],
})
var model Model
model.ProjectId = types.StringValue(idParts[0])
model.ZoneId = types.StringValue(idParts[1])
model.RecordSetId = types.StringValue(idParts[2])
model.Id = utils.BuildInternalTerraformId(idParts[0], idParts[1], idParts[2])

if err := utils.SetModelFieldsToNull(ctx, &model); err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error importing zone", fmt.Sprintf("Setting model fields to null: %v", err))
return
}

diags := resp.State.Set(ctx, model)
resp.Diagnostics.Append(diags...)
if diags.HasError() {
return
}

tflog.Info(ctx, "DNS record set state imported")
}

Expand Down
Loading
Loading