ARO-17482 resolve copilot comments

Author: rajdeepc2792

admin/README.md

@@ -18,7 +18,7 @@ This prefix is abbreviated as `{resourceId}` below.
 |--------|------|-------------|
 | `PUT` | `/admin/v1/hcp{resourceId}/breakglass?group=...&ttl=...` | Create a breakglass session ([details](breakglass.md)) |
 | `GET` | `/admin/v1/hcp{resourceId}/breakglass/{sessionName}/kubeconfig` | Get kubeconfig for a breakglass session ([details](breakglass.md)) |
-| `GET` | `/admin/v1/hcp{resourceId}/serialconsole?vmName=...` | Retrieve serial console logs for a VM ([details](serialconsole.md)) |
+| `GET` | `/admin/v1/hcp{resourceId}/serialconsole?vmName=...` | Retrieve serial console logs for a VM |
 | `GET` | `/admin/v1/hcp{resourceId}/cosmosdump` | Cosmos DB dump for a cluster |
 | `GET` | `/admin/v1/hcp{resourceId}/helloworld` | HCP hello world (dev/test) |
 | `GET` | `/admin/helloworld` | Hello world (dev/test) |

admin/server/go.mod

@@ -58,7 +58,6 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/css v1.0.1 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect
-	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/jedib0t/go-pretty/v6 v6.6.7 // indirect
 	github.com/josharian/intern v1.0.0 // indirect

admin/server/go.sum

@@ -106,8 +106,6 @@ github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8=
 github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 h1:NmZ1PKzSTQbuGHw9DGPFomqkkLWMC+vZCkfs+FHv1Vg=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3/go.mod h1:zQrxl1YP88HQlA6i9c63DSVPFklWpGX4OWAc9bFuaH4=
-github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY=
-github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/itchyny/gojq v0.12.7 h1:hYPTpeWfrJ1OT+2j6cvBScbhl0TkdwGM4bc66onUSOQ=

admin/server/handlers/hcp/serialconsole.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
 
@@ -158,8 +159,10 @@ func (h *HCPSerialConsoleHandler) ServeHTTP(writer http.ResponseWriter, request
 		return fmt.Errorf("failed to create blob request: %w", err)
 	}
 
-	// download blob content
-	httpClient := &http.Client{}
+	// download blob content with timeout to avoid stuck handlers on slow blob endpoints
+	httpClient := &http.Client{
+		Timeout: 30 * time.Second,
+	}
 	blobResp, err := httpClient.Do(blobReq)
 	if err != nil {
 		return fmt.Errorf("failed to download serial console log: %w", err)
@@ -170,13 +173,19 @@ func (h *HCPSerialConsoleHandler) ServeHTTP(writer http.ResponseWriter, request
 		return fmt.Errorf("failed to download serial console log: unexpected status %d", blobResp.StatusCode)
 	}
 
-	// stream response as text/plain
+	// stream response as text/plain and prevent caching of potentially sensitive console output
 	writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	writer.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
+	writer.Header().Set("Pragma", "no-cache")
+	writer.Header().Set("Expires", "0")
 	writer.WriteHeader(http.StatusOK)
 	_, err = io.Copy(writer, blobResp.Body)
 	if err != nil {
-		// If we fail during streaming, log it
-		return fmt.Errorf("failed to stream serial console log: %w", err)
+		// After headers are sent, we cannot return an error response
+		// Log the error and return nil to avoid panic
+		logger := utils.LoggerFromContext(request.Context())
+		logger.Error(err, "failed to stream serial console log", "vmName", vmName)
+		return nil
 	}
 
 	return nil

test/util/framework/admin_api_helpers.go

@@ -409,18 +409,22 @@ func (tc *perItOrDescribeTestContext) GetSerialConsoleLogs(ctx context.Context,
 
 	adminAPIEndpoint := tc.perBinaryInvocationTestContext.adminAPIAddress
 
-	serialConsoleEndpoint := fmt.Sprintf("%s/admin/v1/hcp%s/serialconsole?vmName=%s",
+	serialConsoleEndpoint := fmt.Sprintf("%s/admin/v1/hcp%s/serialconsole",
 		adminAPIEndpoint,
 		resourceID,
-		vmName,
 	)
 
-	By(fmt.Sprintf("reaching out to the admin API to retrieve serial console logs for VM %s: %s", vmName, serialConsoleEndpoint))
+	By(fmt.Sprintf("reaching out to the admin API to retrieve serial console logs for VM %s", vmName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, serialConsoleEndpoint, nil)
 	if err != nil {
 		return "", fmt.Errorf("failed to create request: %w", err)
 	}
 
+	// Add query parameter with proper encoding
+	q := req.URL.Query()
+	q.Add("vmName", vmName)
+	req.URL.RawQuery = q.Encode()
+
 	resp, err := httpClient.Do(req)
 	if err != nil {
 		return "", fmt.Errorf("failed to send request: %w", err)