Skip to content

Conversation

maxbischoff
Copy link
Contributor

@maxbischoff maxbischoff commented Sep 18, 2025

Description

We want to manage our kubeconfigs with terraform and then use them for deploying resources to kubernetes.
Since we don't constantly re-apply the terraform-module creating the kubeconfig, we often run into situations where the kubeconfig has expired, causing failures in our deployment, but the last terraform apply hasn't refreshed the config yet.

With this feature we can ensure that a new kubeconfig will be created before the old one expires.

Checklist

  • Issue was linked above
  • Code format was applied: make fmt
  • Examples were added / adjusted (see examples/ directory)
  • Docs are up-to-date: make generate-docs (will be checked by CI)
  • Unit tests got implemented or updated
  • Acceptance tests got implemented or updated (see e.g. here)
  • Unit tests are passing: make test (will be checked by CI)
  • No linter issues: make lint (will be checked by CI)

@maxbischoff maxbischoff requested a review from a team as a code owner September 18, 2025 12:49
@maxbischoff
Copy link
Contributor Author

Can you give me some general feedback whether this feature is desirable, and if you are happy with the naming of the new field?
If yes I can also add examples and acceptance tests.

@rubenhoenle
Copy link
Member

Hi @maxbischoff, the feature looks like a nice improvement to me from a first glance.

Some remarks before starting with the in-depth review:

Maybe you can adjust the example to show off your new feature:

resource "stackit_ske_kubeconfig" "example" {
project_id = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
cluster_name = "example-cluster"
refresh = true
}

Please also adjust the "Max" acceptance test to include your new field:

// Kubeconfig
resource.TestCheckResourceAttrPair(
"stackit_ske_kubeconfig.kubeconfig", "project_id",
"stackit_ske_cluster.cluster", "project_id",
),

@maxbischoff
Copy link
Contributor Author

Thanks for the quick feedback @rubenhoenle , I added it to the example and the acceptance test.

@rubenhoenle
Copy link
Member

@maxbischoff Sadly the acc tests are failing, see below


➜  terraform-provider-stackit git:(cd877d73) TF_ACC=1 go test -timeout=60m ./stackit/internal/services/ske/ske_acc_test.go
go: downloading github.com/hashicorp/terraform-plugin-go v0.29.0
go: downloading golang.org/x/crypto v0.42.0
go: downloading github.com/hashicorp/go-plugin v1.7.0
go: downloading google.golang.org/protobuf v1.36.9
go: downloading google.golang.org/grpc v1.75.1
go: downloading github.com/hashicorp/terraform-registry-address v0.4.0
go: downloading google.golang.org/genproto/googleapis/rpc v0.0.0-20250707201910-8d1bb00bc6a7
--- FAIL: TestAccSKEMax (862.89s)
    ske_acc_test.go:234: Step 1/4 error: Check failed: Check 50/51 error: stackit_ske_kubeconfig.kubeconfig: Attribute 'refresh_before' not found
FAIL
FAIL    command-line-arguments  2236.203s
FAIL

How to fix?

I guess you will have to add your new attribute here:

resource "stackit_ske_kubeconfig" "kubeconfig" {
project_id = stackit_ske_cluster.cluster.project_id
cluster_name = stackit_ske_cluster.cluster.name
expiration = var.expiration
refresh = var.refresh
}

And don't forget do add a new variable for it:

variable "expiration" {}
variable "refresh" {}


Some background info why I'm asking you to do this: The resource-max.tf file is used in the acceptance test via an embedding, see below

//go:embed testdata/resource-max.tf
resourceMax string

@rubenhoenle rubenhoenle added the needs-work PR needs changes by the author. label Sep 22, 2025
@rubenhoenle
Copy link
Member

Please also check the failing CI pipeline @maxbischoff

@maxbischoff maxbischoff force-pushed the ske-kubeconfig-refresh-time branch from cd877d7 to 80e60d1 Compare September 22, 2025 11:54
@maxbischoff
Copy link
Contributor Author

Sorry about the Acceptance Tests, I though I ran them locally, but there was a make target that I would have needed to use for that. I currently don't have access to a STACKIT project for testing the change, but I think everything should be good now.

@rubenhoenle
Copy link
Member

Sorry about the Acceptance Tests, I though I ran them locally, but there was a make target that I would have needed to use for that. I currently don't have access to a STACKIT project for testing the change, but I think everything should be good now.

No worries, it's not intended that you have to run them (at least for such a small change). I will re-run them tomorrow, they run for some time 😄

@rubenhoenle rubenhoenle removed the needs-work PR needs changes by the author. label Sep 23, 2025
@rubenhoenle
Copy link
Member

Acc tests look good now 😊

➜  terraform-provider-stackit git:(80e60d19) TF_ACC=1 go test -timeout=60m ./stackit/internal/services/ske/ske_acc_test.go
ok      command-line-arguments  2739.181s

rubenhoenle
rubenhoenle previously approved these changes Sep 23, 2025
@rubenhoenle rubenhoenle added the needs-work PR needs changes by the author. label Sep 29, 2025
@maxbischoff maxbischoff force-pushed the ske-kubeconfig-refresh-time branch from 60b6d0c to 48e1ccb Compare October 2, 2025 10:04
@rubenhoenle rubenhoenle enabled auto-merge (squash) October 6, 2025 08:49
@rubenhoenle rubenhoenle merged commit 0763a5f into stackitcloud:main Oct 6, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has internal tracking issue needs-work PR needs changes by the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants