Skip to content

Conversation

@h3adex
Copy link
Contributor

@h3adex h3adex commented Nov 21, 2025

Description

Internal Issue: STACKITTPR-353

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)

Signed-off-by: Mauritz Uphoff <mauritz.uphoff@stackit.cloud>
@h3adex h3adex requested a review from a team as a code owner November 21, 2025 13:49
RoundTripper http.RoundTripper
ServiceAccountEmail string // Deprecated: ServiceAccountEmail is not required and will be removed after 12th June 2025.

PrivateKey string
Copy link
Member

Choose a reason for hiding this comment

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

Could we have a seperate struct for that? E.g. by using https://gobyexample.com/struct-embedding

type EphermalProviderData struct {
    ProviderData
    
    PrivateKey            string
    PrivateKeyPath        string
	ServiceAccountKey     string
	ServiceAccountKeyPath string
}

The fields like PrivateKey, ... don't belong into regular resources and datasources. You don't even set them there (what is good of course 😅). So some abstraction won't hurt here to keep things clean.

return
}

ctx = tflog.SetField(ctx, "access_token", accessToken)
Copy link
Member

Choose a reason for hiding this comment

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

This is a HUGE no no, please don't ever (!!) log credentials

func resolvePrivateKey(ctx context.Context, cfgValue, cfgPath string, key *clients.ServiceAccountKeyResponse) (string, diag.Diagnostics) {
var diags diag.Diagnostics

env := os.Getenv("STACKIT_PRIVATE_KEY")
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be done already by the provider logic? I'm not willing to read env variables here...

Copy link
Member

Choose a reason for hiding this comment

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

If it's not done there, it belongs there. This doesn't belong into some resource implementation for sure.

func loadServiceAccountKey(ctx context.Context, cfgValue, cfgPath string) (*clients.ServiceAccountKeyResponse, diag.Diagnostics) {
var diags diag.Diagnostics

env := os.Getenv("STACKIT_SERVICE_ACCOUNT_KEY")
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be done already by the provider logic? I'm not willing to read env variables here...

)

// #nosec G101 tokenUrl is a public endpoint, not a hardcoded credential
const tokenUrl = "https://service-account.api.stackit.cloud/token"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Other question, what's with custom endpoints?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants