Skip to content

Conversation

JSpon
Copy link

@JSpon JSpon commented Dec 18, 2024

Add registry credentials to cks creation, and caàtch failures on creation on for better state

@CodeBleu CodeBleu added the enhancement New feature or request label Dec 18, 2024
@CodeBleu CodeBleu added this to the v0.6.0 milestone Dec 18, 2024
@CodeBleu
Copy link
Collaborator

@JSpon Thanks for your contribution. Can you please add tests for this? It's also good to follow this once you've added your tests as this can help catch things before the github workflow runs it.

@kiranchavala
Copy link
Collaborator

@kiranchavala kiranchavala reopened this Aug 29, 2025
@vishesh92 vishesh92 requested a review from Copilot September 2, 2025 08:05
@vishesh92
Copy link
Member

@JSpon Please you update the documentation as well.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Docker registry credential support to CloudStack Kubernetes cluster creation and improves error handling by attempting to retrieve cluster information when creation fails.

  • Adds three new optional schema fields for Docker registry credentials (URL, username, password)
  • Sets Docker registry parameters in the creation request when provided
  • Implements error recovery by attempting to retrieve cluster information on creation failure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +220 to +226
cluster, _, errg := cs.Kubernetes.GetKubernetesClusterByName(
name,
cloudstack.WithProject(d.Get("project").(string)),
)
if errg == nil {
d.SetId(cluster.Id)
}
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Setting the resource ID on creation failure is incorrect. This creates inconsistent state where Terraform believes the resource was created successfully but the actual creation failed. The original error should be returned without setting the ID, allowing Terraform to properly handle the failed creation.

Suggested change
cluster, _, errg := cs.Kubernetes.GetKubernetesClusterByName(
name,
cloudstack.WithProject(d.Get("project").(string)),
)
if errg == nil {
d.SetId(cluster.Id)
}

Copilot uses AI. Check for mistakes.

Comment on lines +148 to +152
"docker_registry_password": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Sensitive: true,
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The docker_registry_password field should include DiffSuppressOnRefresh: true to prevent sensitive values from being stored in state and logged during refresh operations.

Suggested change
"docker_registry_password": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Sensitive: true,
Sensitive: true,
DiffSuppressOnRefresh: true,

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants