address review comments
diff --git a/cloudstack/data_source_cloudstack_kubernetes_cluster_config.go b/cloudstack/data_source_cloudstack_kubernetes_cluster_config.go index a2ce623..3116aca 100644 --- a/cloudstack/data_source_cloudstack_kubernetes_cluster_config.go +++ b/cloudstack/data_source_cloudstack_kubernetes_cluster_config.go
@@ -157,8 +157,8 @@ } if *credentials == (kubernetesClusterCredentials{}) { - return fmt.Errorf("The config of Kubernetes Cluster %s does not contain a cluster endpoint, "+ - "CA certificate, client certificate or client key; use config_data directly instead", clusterID) + log.Printf("[WARN] Could not derive a cluster endpoint, CA certificate, client certificate or "+ + "client key from the config of Kubernetes Cluster %s; use config_data directly instead", clusterID) } d.SetId(config.Id) @@ -172,26 +172,25 @@ return nil } -// parseKubernetesClusterConfig extracts the endpoint and the client credentials -// from a kubeconfig document. An unparsable document, an undecodable -// certificate, or a current context that names a cluster/user absent from a -// multi-entry list is an error; a kubeconfig that simply does not carry a -// cluster or a user entry at all only yields empty values, so that config_data -// stays usable. +// parseKubernetesClusterConfig extracts the endpoint and client credentials +// from a kubeconfig document. func parseKubernetesClusterConfig(configData string) (*kubernetesClusterCredentials, error) { var config kubeConfig if err := yaml.Unmarshal([]byte(configData), &config); err != nil { return nil, fmt.Errorf("Invalid kubeconfig: %s", err) } - // Resolve which cluster and user the current context points at. + // The current context is resolved the same way as the cluster/user it names. + contextIndex, err := findKubeConfigEntry(config.Contexts, config.CurrentContext, "context", + func(c kubeConfigContext) string { return c.Name }) + if err != nil { + return nil, err + } + clusterName, userName := "", "" - for _, context := range config.Contexts { - if context.Name == config.CurrentContext { - clusterName = context.Context.Cluster - userName = context.Context.User - break - } + if contextIndex >= 0 { + clusterName = config.Contexts[contextIndex].Context.Cluster + userName = config.Contexts[contextIndex].Context.User } credentials := &kubernetesClusterCredentials{} @@ -237,15 +236,9 @@ return credentials, nil } -// findKubeConfigEntry returns the index of the named entry in a kubeconfig -// list. It returns -1 with no error when the list is empty, so that a -// kubeconfig which does not carry the entry at all still leaves config_data -// usable. When the current context does not name an entry at all, or there is -// only one entry to begin with, it falls back to the first one, since there is -// only one reasonable candidate either way. But when the current context names -// a specific entry that is absent from a list of more than one, guessing is -// refused: picking some other entry could silently pair one cluster's endpoint -// with a different cluster's client credentials. +// findKubeConfigEntry finds an entry by name, falling back to the sole entry +// when there's only one. With more than one and no match, it errors instead +// of guessing, since a wrong guess could pair the wrong cluster and user. func findKubeConfigEntry[T any](entries []T, name string, kind string, nameOf func(T) string) (int, error) { if len(entries) == 0 { log.Printf("[WARN] Kubeconfig does not contain any %s", kind) @@ -258,11 +251,11 @@ } if len(entries) > 1 { - return -1, fmt.Errorf("kubeconfig context references %s %q, which is not defined among %d %s entries", + return -1, fmt.Errorf("kubeconfig does not define %s %q among its %d %s entries", kind, name, len(entries), kind) } - log.Printf("[WARN] Kubeconfig does not contain the %s %q referenced by its current context, using the only %s instead", kind, name, kind) + log.Printf("[WARN] Kubeconfig does not define the %s %q, using the only %s available instead", kind, name, kind) return 0, nil }
diff --git a/cloudstack/data_source_cloudstack_kubernetes_cluster_config_test.go b/cloudstack/data_source_cloudstack_kubernetes_cluster_config_test.go index 8365a3f..6cfe0eb 100644 --- a/cloudstack/data_source_cloudstack_kubernetes_cluster_config_test.go +++ b/cloudstack/data_source_cloudstack_kubernetes_cluster_config_test.go
@@ -84,20 +84,13 @@ }, }, { - name: "unknown current context falls back to the first entry", + // Ambiguous among several contexts: error rather than guess. + name: "unknown current context among several contexts is an error", configData: testKubeConfigTwoClusters("missing@missing"), - expected: kubernetesClusterCredentials{ - Endpoint: "https://10.1.1.1:6443", - ClusterCACertificate: "first-ca-certificate", - ClientCertificate: "first-client-certificate", - ClientKey: "first-client-key", - }, + expectErr: "does not define", }, { - // The context resolves, but names a cluster and user that the - // kubeconfig does not define. With only one entry of each, - // there's only one reasonable candidate, so this still falls - // back rather than erroring. + // Only one candidate exists, so this falls back instead of erroring. name: "current context naming a missing cluster falls back to the only entry", configData: fmt.Sprintf(`apiVersion: v1 kind: Config @@ -127,12 +120,8 @@ }, }, { - // Here the context resolves and names a cluster absent from a - // list of more than one: guessing which of the two is intended - // could silently pair the wrong cluster/user together, so this - // must be a hard error instead of a silent fallback. Clusters are - // resolved before users, so no users section is needed to reach - // this error. + // Ambiguous among several clusters: error rather than guess. + // No users section needed; the cluster error returns first. name: "current context naming an undefined cluster among several is an error", configData: fmt.Sprintf(`apiVersion: v1 kind: Config @@ -152,13 +141,10 @@ user: third name: third@third `, testBase64("first-ca-certificate"), testBase64("second-ca-certificate")), - expectErr: "which is not defined", + expectErr: "does not define", }, { - // The same hard error, but reached through the user branch - // instead of the cluster branch, so a future edit that swaps - // clusterName/userName or config.Clusters/config.Users between - // the two findKubeConfigEntry call sites would still be caught. + // Same error, but via the user branch instead of the cluster branch. name: "current context naming an undefined user among several is an error", configData: fmt.Sprintf(`apiVersion: v1 kind: Config @@ -185,7 +171,7 @@ `, testBase64("ca-certificate"), testBase64("first-client-certificate"), testBase64("first-client-key"), testBase64("second-client-certificate"), testBase64("second-client-key")), - expectErr: "which is not defined", + expectErr: "does not define", }, { // A kubeconfig without client certificates must not fail the data @@ -284,12 +270,9 @@ return base64.StdEncoding.EncodeToString([]byte(value)) } -// testKubeConfigTwoClusters renders a kubeconfig holding two clusters and two -// users, so that context resolution can be told apart from taking the first -// entry. currentContext selects which one context resolution should pick; an -// empty string omits the current-context key entirely. Every %s verb appears -// in the same order as its argument below, so the substitution can be checked -// by reading both top to bottom in lockstep. +// testKubeConfigTwoClusters renders a two-cluster, two-user kubeconfig; +// currentContext picks the context ("" omits current-context). Args are +// listed in template order so the substitution can be eyeballed. func testKubeConfigTwoClusters(currentContext string) string { return fmt.Sprintf(`apiVersion: v1 kind: Config