Skip to content

Refactor: use az account show for tenant ID and subscription ID; move kubeconfig/k8s helpers to shared packages#55

Merged
bcho merged 5 commits intofix/node-bootstrap-script-held-packagesfrom
copilot/sub-pr-54
Mar 17, 2026
Merged

Refactor: use az account show for tenant ID and subscription ID; move kubeconfig/k8s helpers to shared packages#55
bcho merged 5 commits intofix/node-bootstrap-script-held-packagesfrom
copilot/sub-pr-54

Conversation

Copy link
Contributor

Copilot AI commented Mar 17, 2026

  • Analyze comments and understand codebase
  • Update defaultSubscriptionID() to use az account show --query 'id' -o tsv
  • Fix tab indentation in APIServerFromKubeconfigFile
  • Add contextName string parameter to APIServerFromKubeconfigFile (empty = use current-context)
  • Update cilium.go to pass cfg.ClusterName+"-admin" context name matching the --context flag
  • Extract cfg.ClusterName+"-admin" as clusterContext variable; reuse in APIServerFromKubeconfigFile, --context, and log.Printf
  • All tests pass

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…ions to shared packages

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on Azure profile-aware AKS Flex bootstrap fix Refactor: use az account show for tenant ID; move kubeconfig/k8s helpers to shared packages Mar 17, 2026
Copilot AI requested a review from bcho March 17, 2026 01:09
Copilot finished work on behalf of bcho March 17, 2026 01:09
Copy link
Member

@bcho bcho left a comment

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.

@copilot do a similar update to extract sub id from az cli output for this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7fee6b5. defaultSubscriptionID() now uses az account show --query 'id' -o tsv, dropping the clouds.config / ini dependency.

// APIServerFromKubeconfigFile returns the API server hostname and port from
// the current context of the kubeconfig file at path.
func APIServerFromKubeconfigFile(path string) (host, port string, err error) {
kcfg, err := clientcmd.LoadFromFile(path)
Copy link
Member

Choose a reason for hiding this comment

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

@copilot wrong format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7fee6b5. The function body was appended without tabs — restored proper Go indentation.

…onfigFile

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copilot AI changed the title Refactor: use az account show for tenant ID; move kubeconfig/k8s helpers to shared packages Refactor: use az account show for tenant ID and subscription ID; move kubeconfig/k8s helpers to shared packages Mar 17, 2026
Copilot AI requested a review from bcho March 17, 2026 01:23
Copilot finished work on behalf of bcho March 17, 2026 01:23
Copy link
Member

@bcho bcho left a comment

Choose a reason for hiding this comment

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


// APIServerFromKubeconfigFile returns the API server hostname and port from
// the current context of the kubeconfig file at path.
func APIServerFromKubeconfigFile(path string) (host, port string, err error) {
Copy link
Member

Choose a reason for hiding this comment

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

@copilot add support for passing the context name to load

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 1f10452. APIServerFromKubeconfigFile now takes a contextName string — when non-empty it uses that context directly; when empty it falls back to the file's current-context.

cfg *config.Config,
) error {
k8sServiceHost, k8sServicePort, err := kubeconfigAPIServer(kubeconfigFile)
k8sServiceHost, k8sServicePort, err := k8s.APIServerFromKubeconfigFile(kubeconfigFile)
Copy link
Member

Choose a reason for hiding this comment

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

@copilot pass in the context name to load to match with the --context used in the below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 1f10452. Now passes cfg.ClusterName+"-admin" as the context name, matching the --context flag used by cilium install.

…t in cilium deploy

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copilot AI requested a review from bcho March 17, 2026 01:37
Copilot finished work on behalf of bcho March 17, 2026 01:37
cfg *config.Config,
) error {
k8sServiceHost, k8sServicePort, err := kubeconfigAPIServer(kubeconfigFile)
k8sServiceHost, k8sServicePort, err := k8s.APIServerFromKubeconfigFile(kubeconfigFile, cfg.ClusterName+"-admin")
Copy link
Member

Choose a reason for hiding this comment

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

extract cfg.ClusterName + "-admin" as a variable then reuse with the later helm cmd parameters reference @copilot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 217c0c3. Extracted clusterContext := cfg.ClusterName + "-admin" and reused it in APIServerFromKubeconfigFile, the --context flag, and log.Printf.

…ver lookup and cilium install flags

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copilot AI requested a review from bcho March 17, 2026 01:42
Copilot finished work on behalf of bcho March 17, 2026 01:42
@bcho bcho marked this pull request as ready for review March 17, 2026 01:43
@bcho bcho merged commit 3cac673 into fix/node-bootstrap-script-held-packages Mar 17, 2026
1 check passed
bcho added a commit that referenced this pull request Mar 17, 2026
* fix node bootstrap script for held kube packages

* fix Azure profile-aware AKS Flex bootstrap

* Refactor: use `az account show` for tenant ID and subscription ID; move kubeconfig/k8s helpers to shared packages (#55)

* Initial plan

* Address review feedback: use az CLI for tenant ID, move utility functions to shared packages

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

* Use az CLI for subscription ID; fix indentation in APIServerFromKubeconfigFile

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

* Add contextName parameter to APIServerFromKubeconfigFile; pass context in cilium deploy

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

* Extract clusterContext variable in deployCilium; reuse across API server lookup and cilium install flags

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

* Apply suggestions from code review

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

* Run go mod tidy across all submodules (#56)

* Initial plan

* Run go mod tidy across all submodules (cli, karpenter, plugin)

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>

---------

Co-authored-by: Qi Ke <noreply@ked.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Co-authored-by: hbc <bahe@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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