support for private AKS clusters by devdoxxx · Pull Request #181 · Azure/k8s-create-secret

devdoxxx · 2025-07-29T18:08:17Z

This PR introduces full support for deploying Kubernetes secrets to private AKS clusters, which do not expose a public API server. It enables secure secret creation by leveraging az aks command invoke to execute kubectl commands directly within the cluster's internal context.

Key Features
Private Cluster Support: Uses az aks command invoke to apply or delete secrets within private AKS clusters.

Fallback to Kubernetes API: Maintains existing functionality for public clusters via the Kubernetes API client.

Temporary Secret File Handling: Writes secrets to a JSON temp file and applies them securely via kubectl apply -f.

Robust Cleanup: Ensures the temporary file is removed after execution, even if an error occurs.

Input-Driven Control: Introduces use-invoke-command input to toggle between public/private cluster deployment modes.

It also includes full unit test coverage for the private cluster pathway and related helper functions.

This addresses #82

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR adds support for deploying Kubernetes secrets to private AKS clusters by introducing an alternative execution path that uses az aks command invoke instead of direct API calls.

Adds runKubectlViaAz function to execute kubectl commands via Azure CLI for private clusters
Introduces conditional logic in the main run function to use the new invoke command when enabled
Adds three new input parameters to control private cluster deployment mode

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/run.ts	Implements core functionality for private cluster support with temp file handling and Azure CLI execution
action.yml	Adds input parameters for controlling private cluster mode and cluster identification
test/run.test.ts	Adds comprehensive unit tests for the new private cluster functionality

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

Copilot · 2025-08-12T21:22:28Z

+      )
+   }
+   // Write secret to temp file
+   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)


This line creates a tempFile variable that is immediately overwritten on line 74. This dead code should be removed as it serves no purpose and could cause confusion.

Suggested change

const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

Copilot · 2025-08-12T21:22:28Z

+      )
+   }
+   // Write secret to temp file
+   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)


This line redeclares the tempFile variable that was already declared on line 71, which will cause a compilation error in TypeScript due to duplicate variable declaration in the same scope.

Suggested change

const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

Copilot · 2025-08-12T21:22:29Z

+         '--name',
+         clusterName,
+         '--command',
+         `kubectl apply -f - -n ${namespace}`,


The kubectl command uses -f - (stdin) but then specifies --file parameter with the temp file path. This is inconsistent - either use -f - with stdin or -f ${tempFile} with the file parameter, but not both.

Suggested change

`kubectl apply -f - -n ${namespace}`,

`kubectl apply -f ${secretName}.json -n ${namespace}`,

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/run.ts:71

The initial value of tempFile is unused, since it is always overwritten.

   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T15:41:31Z

+   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)
+   // Write secret to temp file securely
+   const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'secret-'));


The variable tempDir is declared on line 73 but never has a corresponding declaration statement. This will cause a reference error since tempDir is used in the finally block (lines 107-109) but is only accessible within the scope after line 73. The tempDir variable needs to be declared before line 73 (e.g., let tempDir: string;) to be accessible in the finally block.

Suggested change

const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

// Write secret to temp file securely

const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'secret-'));

let tempDir: string;

// Write secret to temp file securely

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'secret-'));

Copilot · 2025-12-08T15:41:32Z

+   const sec = await buildSecret(secretName, namespace, secretType)
+
+   if (core.getInput('use-invoke-command') === 'true') {
+      await runKubectlViaAz(sec, namespace, secretName)
+      return
+   }


The checkClusterContext() function is still called unconditionally at the start of run(), which checks for the KUBECONFIG environment variable. However, when use-invoke-command is true, the KUBECONFIG is not needed since the action uses az aks command invoke instead. This will cause the action to fail for private clusters even when the invoke command path is selected. Consider making this check conditional or moving it to only execute when the Kubernetes API path is used.

Copilot · 2025-12-08T15:41:32Z

+   beforeEach(() => {
+      jest.spyOn(fs, 'writeFileSync').mockImplementation(() => {})
+      jest.spyOn(fs, 'unlinkSync').mockImplementation(() => {})
+      jest.spyOn(exec, 'exec').mockResolvedValue(0)
+   })


The test is missing mocks for fs.mkdtempSync, fs.existsSync, and fs.rmSync which are called in the runKubectlViaAz function (lines 73, 103, 107-108 in src/run.ts). Without these mocks, the tests may attempt to create/delete actual temporary directories on the filesystem during test execution. Add mocks for these methods in the beforeEach block.

Copilot · 2025-12-08T15:41:32Z

+   it('calls az aks command invoke for delete and apply', async () => {
+      jest
+         .spyOn(core, 'getInput')
+         .mockImplementation((name: string) =>
+            name === 'cluster-resource-group'
+               ? 'rg'
+               : name === 'cluster-name'
+                 ? 'aks'
+                 : ''
+         )
+      const {runKubectlViaAz} = require('../src/run')
+      await runKubectlViaAz(mockSecret, 'test-ns', 'test-secret')
+      expect(exec.exec).toHaveBeenCalledWith(
+         'az',
+         expect.arrayContaining([
+            'aks',
+            'command',
+            'invoke',
+            '--resource-group',
+            'rg',
+            '--name',
+            'aks'
+         ])
+      )
+   })


The test only verifies that exec.exec was called with certain arguments but doesn't verify:

That it was called exactly twice (once for delete, once for apply)

The complete command arguments including the kubectl commands

That the temp file operations occurred in the correct order

Consider using toHaveBeenCalledTimes(2) and more specific matchers to verify both the delete and apply commands were executed with the correct full argument lists.

Copilot · 2025-12-08T15:41:33Z

+      // Remove temp file if it exists
+      if (fs.existsSync(tempFile)) {
+         fs.unlinkSync(tempFile)
+      }


The cleanup logic is redundant. Lines 103-105 attempt to delete tempFile individually, but lines 107-109 already delete the entire tempDir recursively with { recursive: true, force: true }, which would remove tempFile as well. The individual file deletion (lines 103-105) is unnecessary and can be removed.

Suggested change

// Remove temp file if it exists

if (fs.existsSync(tempFile)) {

fs.unlinkSync(tempFile)

}

Copilot · 2025-12-08T15:41:33Z

+   // Write secret to temp file
+   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)


Assignment to variable tempFile, which is declared constant.

Suggested change

// Write secret to temp file

const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

Copilot · 2025-12-08T15:41:33Z

+   // Write secret to temp file
+   const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)


Assignment to variable tempFile, which is declared constant.

Suggested change

// Write secret to temp file

const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

support for private AKS clusters

2193f7f

devdoxxx marked this pull request as ready for review July 29, 2025 18:12

devdoxxx requested a review from a team as a code owner July 29, 2025 18:12

devdoxxx requested a review from Copilot July 29, 2025 18:12

devdoxxx self-assigned this Jul 29, 2025

This comment was marked as outdated.

Sign in to view

ReinierCC requested a review from Copilot August 12, 2025 19:24

This comment was marked as outdated.

Sign in to view

Update src/run.ts

46a8c3a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ReinierCC requested a review from Copilot August 12, 2025 21:04

This comment was marked as outdated.

Sign in to view

ReinierCC and others added 2 commits August 12, 2025 17:10

Update src/run.ts

71e1bdd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/run.ts

c279b27

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ReinierCC requested a review from Copilot August 12, 2025 21:21

Copilot AI reviewed Aug 12, 2025

View reviewed changes

ReinierCC requested a review from Copilot December 8, 2025 15:36

Copilot started reviewing on behalf of ReinierCC December 8, 2025 15:36 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

	`kubectl apply -f - -n ${namespace}`,
	`kubectl apply -f ${secretName}.json -n ${namespace}`,

		// Write secret to temp file
		const tempFile = path.join(os.tmpdir(), `secret-${secretName}.json`)

Conversation

devdoxxx commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

devdoxxx commented Jul 29, 2025 •

edited

Loading