feat: add operator workflows - #692
Conversation
1d54c0a to
7d02e73
Compare
There was a problem hiding this comment.
11 issues found across 16 files
Confidence score: 2/5
cmd/demo2_up.goreuses an existing cluster by name without confirming it is the local demo resource, sodemo2 upcould reconfigure or deploy to an unrelated managed cluster — require local-resource validation before reuse.cmd/demo2_up_orchestrator.govalidates against the current kubeconfig context rather than selecting the named k3d cluster, which can report success for the wrong cluster — switch to the demo cluster context before validation.cmd/demo2_destroy.godeletes local resources before removing the remote cluster configuration, leaving remote state behind if deletion fails — delete the remote configuration first or add reliable rollback/retry handling.cmd/cluster_operator_status.goreports resolution and request failures but exits successfully, so automation can treat a failed status check as success — return a nonzero exit after these errors.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmd/demo2_up_orchestrator.go">
<violation number="1" location="cmd/demo2_up_orchestrator.go:126">
P3: The success message printed after a completed installation contains a literal, unsubstituted placeholder "use k3d cluster xxxx" instead of the real cluster name. After a successful demo install, users are told to run a non-existent literal command "k3d cluster xxxx", which is misleading. Substitute the actual cluster name (the same value already passed as the %q argument).</violation>
<violation number="2" location="cmd/demo2_up_orchestrator.go:212">
P1: When the named k3d cluster already exists and another kubeconfig context is current, this validation inspects the wrong cluster and can report success incorrectly. Select the demo cluster context before validating workloads, or pass the context explicitly to `ValidateWorkloads`.</violation>
</file>
<file name="cmd/demo2_up.go">
<violation number="1" location="cmd/demo2_up.go:224">
P2: When two `demo2 up` attempts run concurrently, this fixed `O_TRUNC` path causes each attempt to overwrite the other’s diagnostics. Create a unique log file per attempt and upload that attempt-specific path.</violation>
<violation number="2" location="cmd/demo2_up.go:287">
P1: When `--cluster-name` matches an existing organization cluster, `FindCluster` reuses it without verifying that it is the local demo resource. The workflow can therefore reconfigure and deploy an unrelated managed or production cluster; restrict reuse to a verified demo cluster or reject non-demo matches.</violation>
<violation number="3" location="cmd/demo2_up.go:681">
P2: When another loopback address shares the `172.42.0.3` prefix, this check falsely reports the required address as configured and skips adding it. Parse the interface addresses and compare the exact IP instead of using substring matching.</violation>
<violation number="4" location="cmd/demo2_up.go:698">
P2: On WSL, `Start-Process` returns before the elevated `netsh` process adds the loopback address, allowing bootstrap and deployment to race with loopback setup. Add `-Wait` to the PowerShell command before returning.</violation>
</file>
<file name="cmd/cluster_operator_status.go">
<violation number="1" location="cmd/cluster_operator_status.go:25">
P2: When organization/cluster resolution or the status request fails, this handler prints the error and returns, so `qovery cluster operator status` exits successfully. Exit nonzero after reporting these errors, as the table-rendering branch already does, so scripts can detect failed status checks.</violation>
</file>
<file name="cmd/cluster_operator_helpers.go">
<violation number="1" location="cmd/cluster_operator_helpers.go:46">
P2: When `--cluster` contains leading or trailing whitespace, `findCluster` reports the cluster as missing even though other cluster commands resolve the same input. Delegate to `utils.FindByClusterName` so operator status uses the existing whitespace-tolerant lookup.</violation>
</file>
<file name="cmd/admin_cluster_operator_list_test.go">
<violation number="1" location="cmd/admin_cluster_operator_list_test.go:16">
P3: `t.Fatalf` is called from inside the `httptest` handler goroutine. The Go testing docs state FailNow (and thus Fatalf) must be called from the goroutine running the test, not from other goroutines; here it can race with the test goroutine and doesn't stop the other goroutines as expected. Return an error from the handler (e.g. write a non-200 status) and assert it on the test goroutine instead, or use `t.Errorf` in the handler followed by a signal the main goroutine can wait on.</violation>
</file>
<file name="cmd/demo2_destroy.go">
<violation number="1" location="cmd/demo2_destroy.go:61">
P3: When the local or remote destroy operation fails, this branch records only the start event and drops the terminal failure telemetry. Emit `CaptureError` in the error branch, as `demo2 up` does, so failed cleanup attempts remain observable.</violation>
<violation number="2" location="cmd/demo2_destroy.go:110">
P2: When `--delete-qovery-config` is set, the local k3d cluster and registry are deleted before the remote Qovery cluster configuration is deleted. If the remote `DeleteClusterConfig` call fails (network error, expired token, API rejection), the local cluster is already gone while the Qovery cluster record remains, leaving an orphaned/broken cluster in the Qovery platform. Deleting the remote configuration first (or making the remote deletion the gate before local teardown) avoids leaving the platform in an inconsistent state.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| if err != nil { | ||
| return "", false, err | ||
| } | ||
| cluster := findCluster(clusters.GetResults(), name) |
There was a problem hiding this comment.
P1: When --cluster-name matches an existing organization cluster, FindCluster reuses it without verifying that it is the local demo resource. The workflow can therefore reconfigure and deploy an unrelated managed or production cluster; restrict reuse to a verified demo cluster or reject non-demo matches.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up.go, line 287:
<comment>When `--cluster-name` matches an existing organization cluster, `FindCluster` reuses it without verifying that it is the local demo resource. The workflow can therefore reconfigure and deploy an unrelated managed or production cluster; restrict reuse to a verified demo cluster or reject non-demo matches.</comment>
<file context>
@@ -0,0 +1,827 @@
+ if err != nil {
+ return "", false, err
+ }
+ cluster := findCluster(clusters.GetResults(), name)
+ if cluster == nil {
+ return "", false, nil
</file context>
| } | ||
|
|
||
| if err := o.runPhase(demo2PhaseWorkloadVerification, "Verifying Operator and Engine workloads", func() error { | ||
| return o.local.ValidateWorkloads(ctx, bootstrap.Namespace) |
There was a problem hiding this comment.
P1: When the named k3d cluster already exists and another kubeconfig context is current, this validation inspects the wrong cluster and can report success incorrectly. Select the demo cluster context before validating workloads, or pass the context explicitly to ValidateWorkloads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up_orchestrator.go, line 212:
<comment>When the named k3d cluster already exists and another kubeconfig context is current, this validation inspects the wrong cluster and can report success incorrectly. Select the demo cluster context before validating workloads, or pass the context explicitly to `ValidateWorkloads`.</comment>
<file context>
@@ -0,0 +1,313 @@
+ }
+
+ if err := o.runPhase(demo2PhaseWorkloadVerification, "Verifying Operator and Engine workloads", func() error {
+ return o.local.ValidateWorkloads(ctx, bootstrap.Namespace)
+ }); err != nil {
+ return err
</file context>
| if err := os.MkdirAll(directory, 0700); err != nil { | ||
| return nil, "", fmt.Errorf("cannot create demo log directory: %w", err) | ||
| } | ||
| logPath := filepath.Join(directory, "qovery-demo.log") |
There was a problem hiding this comment.
P2: When two demo2 up attempts run concurrently, this fixed O_TRUNC path causes each attempt to overwrite the other’s diagnostics. Create a unique log file per attempt and upload that attempt-specific path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up.go, line 224:
<comment>When two `demo2 up` attempts run concurrently, this fixed `O_TRUNC` path causes each attempt to overwrite the other’s diagnostics. Create a unique log file per attempt and upload that attempt-specific path.</comment>
<file context>
@@ -0,0 +1,827 @@
+ if err := os.MkdirAll(directory, 0700); err != nil {
+ return nil, "", fmt.Errorf("cannot create demo log directory: %w", err)
+ }
+ logPath := filepath.Join(directory, "qovery-demo.log")
+ file, err := os.OpenFile(logPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+ if err != nil {
</file context>
| if err != nil { | ||
| return commandFailed("macOS loopback inspection", err) | ||
| } | ||
| if strings.Contains(string(output), demo2NodeIP) { |
There was a problem hiding this comment.
P2: When another loopback address shares the 172.42.0.3 prefix, this check falsely reports the required address as configured and skips adding it. Parse the interface addresses and compare the exact IP instead of using substring matching.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up.go, line 681:
<comment>When another loopback address shares the `172.42.0.3` prefix, this check falsely reports the required address as configured and skips adding it. Parse the interface addresses and compare the exact IP instead of using substring matching.</comment>
<file context>
@@ -0,0 +1,827 @@
+ if err != nil {
+ return commandFailed("macOS loopback inspection", err)
+ }
+ if strings.Contains(string(output), demo2NodeIP) {
+ return nil
+ }
</file context>
| if err := l.runner.LookPath(powershell); err != nil { | ||
| powershell = "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe" | ||
| } | ||
| command := fmt.Sprintf("Start-Process netsh -Verb RunAs -ArgumentList \"interface ipv4 add address name='Loopback Pseudo-Interface 1' address=%s mask=255.255.255.255 skipassource=true\"", demo2NodeIP) |
There was a problem hiding this comment.
P2: On WSL, Start-Process returns before the elevated netsh process adds the loopback address, allowing bootstrap and deployment to race with loopback setup. Add -Wait to the PowerShell command before returning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up.go, line 698:
<comment>On WSL, `Start-Process` returns before the elevated `netsh` process adds the loopback address, allowing bootstrap and deployment to race with loopback setup. Add `-Wait` to the PowerShell command before returning.</comment>
<file context>
@@ -0,0 +1,827 @@
+ if err := l.runner.LookPath(powershell); err != nil {
+ powershell = "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe"
+ }
+ command := fmt.Sprintf("Start-Process netsh -Verb RunAs -ArgumentList \"interface ipv4 add address name='Loopback Pseudo-Interface 1' address=%s mask=255.255.255.255 skipassource=true\"", demo2NodeIP)
+ output, err = l.runner.Run(ctx, powershell, "-NoProfile", "-Command", command)
+ if err != nil && !strings.Contains(strings.ToLower(string(output)), "exists") {
</file context>
| command := fmt.Sprintf("Start-Process netsh -Verb RunAs -ArgumentList \"interface ipv4 add address name='Loopback Pseudo-Interface 1' address=%s mask=255.255.255.255 skipassource=true\"", demo2NodeIP) | |
| command := fmt.Sprintf("Start-Process netsh -Wait -Verb RunAs -ArgumentList \"interface ipv4 add address name='Loopback Pseudo-Interface 1' address=%s mask=255.255.255.255 skipassource=true\"", demo2NodeIP) |
| func findCluster(clusters []qovery.Cluster, name string) *qovery.Cluster { | ||
| for index := range clusters { | ||
| if clusters[index].Name == name { | ||
| return &clusters[index] | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
P2: When --cluster contains leading or trailing whitespace, findCluster reports the cluster as missing even though other cluster commands resolve the same input. Delegate to utils.FindByClusterName so operator status uses the existing whitespace-tolerant lookup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/cluster_operator_helpers.go, line 46:
<comment>When `--cluster` contains leading or trailing whitespace, `findCluster` reports the cluster as missing even though other cluster commands resolve the same input. Delegate to `utils.FindByClusterName` so operator status uses the existing whitespace-tolerant lookup.</comment>
<file context>
@@ -0,0 +1,61 @@
+ }, nil
+}
+
+func findCluster(clusters []qovery.Cluster, name string) *qovery.Cluster {
+ for index := range clusters {
+ if clusters[index].Name == name {
</file context>
| func findCluster(clusters []qovery.Cluster, name string) *qovery.Cluster { | |
| for index := range clusters { | |
| if clusters[index].Name == name { | |
| return &clusters[index] | |
| } | |
| } | |
| return nil | |
| } | |
| func findCluster(clusters []qovery.Cluster, name string) *qovery.Cluster { | |
| return utils.FindByClusterName(clusters, name) | |
| } |
| return fmt.Errorf("local dependency check failed: %w", err) | ||
| } | ||
|
|
||
| o.phase("Deleting the local k3d cluster and registry") |
There was a problem hiding this comment.
P2: When --delete-qovery-config is set, the local k3d cluster and registry are deleted before the remote Qovery cluster configuration is deleted. If the remote DeleteClusterConfig call fails (network error, expired token, API rejection), the local cluster is already gone while the Qovery cluster record remains, leaving an orphaned/broken cluster in the Qovery platform. Deleting the remote configuration first (or making the remote deletion the gate before local teardown) avoids leaving the platform in an inconsistent state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_destroy.go, line 110:
<comment>When `--delete-qovery-config` is set, the local k3d cluster and registry are deleted before the remote Qovery cluster configuration is deleted. If the remote `DeleteClusterConfig` call fails (network error, expired token, API rejection), the local cluster is already gone while the Qovery cluster record remains, leaving an orphaned/broken cluster in the Qovery platform. Deleting the remote configuration first (or making the remote deletion the gate before local teardown) avoids leaving the platform in an inconsistent state.</comment>
<file context>
@@ -0,0 +1,272 @@
+ return fmt.Errorf("local dependency check failed: %w", err)
+ }
+
+ o.phase("Deleting the local k3d cluster and registry")
+ if err := o.local.DeleteK3dCluster(ctx, cfg.ClusterName); err != nil {
+ return fmt.Errorf("cannot delete local k3d resources: %w", err)
</file context>
|
|
||
| var clusterID string | ||
| if err := o.runPhase(demo2PhaseCredentialsAndCluster, "Resolving Qovery On-Premise credentials and cluster", func() error { | ||
| credential, err := o.api.EnsureOnPremiseCredentials(ctx, cfg.OrganizationID) |
There was a problem hiding this comment.
P3: The success message printed after a completed installation contains a literal, unsubstituted placeholder "use k3d cluster xxxx" instead of the real cluster name. After a successful demo install, users are told to run a non-existent literal command "k3d cluster xxxx", which is misleading. Substitute the actual cluster name (the same value already passed as the %q argument).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_up_orchestrator.go, line 126:
<comment>The success message printed after a completed installation contains a literal, unsubstituted placeholder "use k3d cluster xxxx" instead of the real cluster name. After a successful demo install, users are told to run a non-existent literal command "k3d cluster xxxx", which is misleading. Substitute the actual cluster name (the same value already passed as the %q argument).</comment>
<file context>
@@ -0,0 +1,313 @@
+
+ var clusterID string
+ if err := o.runPhase(demo2PhaseCredentialsAndCluster, "Resolving Qovery On-Premise credentials and cluster", func() error {
+ credential, err := o.api.EnsureOnPremiseCredentials(ctx, cfg.OrganizationID)
+ if err != nil {
+ return fmt.Errorf("cannot resolve On-Premise credentials: %w", err)
</file context>
|
|
||
| func TestGetClusterOperatorFleetUsesAdminRoute(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { | ||
| if request.URL.Path != "/operator/clusters" { |
There was a problem hiding this comment.
P3: t.Fatalf is called from inside the httptest handler goroutine. The Go testing docs state FailNow (and thus Fatalf) must be called from the goroutine running the test, not from other goroutines; here it can race with the test goroutine and doesn't stop the other goroutines as expected. Return an error from the handler (e.g. write a non-200 status) and assert it on the test goroutine instead, or use t.Errorf in the handler followed by a signal the main goroutine can wait on.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/admin_cluster_operator_list_test.go, line 16:
<comment>`t.Fatalf` is called from inside the `httptest` handler goroutine. The Go testing docs state FailNow (and thus Fatalf) must be called from the goroutine running the test, not from other goroutines; here it can race with the test goroutine and doesn't stop the other goroutines as expected. Return an error from the handler (e.g. write a non-200 status) and assert it on the test goroutine instead, or use `t.Errorf` in the handler followed by a signal the main goroutine can wait on.</comment>
<file context>
@@ -0,0 +1,100 @@
+
+func TestGetClusterOperatorFleetUsesAdminRoute(t *testing.T) {
+ server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+ if request.URL.Path != "/operator/clusters" {
+ t.Fatalf("unexpected path %s", request.URL.Path)
+ }
</file context>
| if err == nil { | ||
| utils.CaptureWithEvent(cmd, utils.EndOfExecutionEventName) | ||
| } |
There was a problem hiding this comment.
P3: When the local or remote destroy operation fails, this branch records only the start event and drops the terminal failure telemetry. Emit CaptureError in the error branch, as demo2 up does, so failed cleanup attempts remain observable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/demo2_destroy.go, line 61:
<comment>When the local or remote destroy operation fails, this branch records only the start event and drops the terminal failure telemetry. Emit `CaptureError` in the error branch, as `demo2 up` does, so failed cleanup attempts remain observable.</comment>
<file context>
@@ -0,0 +1,272 @@
+ ClusterName: demo2DestroyClusterName,
+ DeleteQoveryConfig: demo2DestroyDeleteQoveryConfig,
+ })
+ if err == nil {
+ utils.CaptureWithEvent(cmd, utils.EndOfExecutionEventName)
+ }
</file context>
| if err == nil { | |
| utils.CaptureWithEvent(cmd, utils.EndOfExecutionEventName) | |
| } | |
| if err == nil { | |
| utils.CaptureWithEvent(cmd, utils.EndOfExecutionEventName) | |
| } else { | |
| utils.CaptureError(cmd, "", err.Error()) | |
| } |
No description provided.