From d105076c99558b37feffe7daddb425f2b05485cc Mon Sep 17 00:00:00 2001 From: Sriram Panyam Date: Mon, 13 Jan 2025 12:13:17 -0800 Subject: [PATCH 1/4] Addon configuration now takes an optional config file to load from instead of forcing prompts Fixes #20124 --- cmd/minikube/cmd/config/configure.go | 616 +++++++++++------- test/integration/addons_test.go | 61 +- .../testdata/addons_testconfig.json | 32 + 3 files changed, 477 insertions(+), 232 deletions(-) create mode 100644 test/integration/testdata/addons_testconfig.json diff --git a/cmd/minikube/cmd/config/configure.go b/cmd/minikube/cmd/config/configure.go index 435dabdb904a..d5039df5e844 100644 --- a/cmd/minikube/cmd/config/configure.go +++ b/cmd/minikube/cmd/config/configure.go @@ -17,8 +17,12 @@ limitations under the License. package config import ( + "encoding/json" + "errors" + "fmt" "net" "os" + "reflect" "regexp" "time" @@ -38,6 +42,7 @@ import ( "k8s.io/minikube/pkg/minikube/sysinit" ) +var AddonConfigFile = "" var posResponses = []string{"yes", "y"} var negResponses = []string{"no", "n"} @@ -51,250 +56,29 @@ var addonsConfigureCmd = &cobra.Command{ } profile := ClusterFlagValue() - addon := args[0] + addonConfig, err := loadAddonConfigFile(addon, AddonConfigFile) + if err != nil { + return + } + // allows for additional prompting of information when enabling addons switch addon { case "registry-creds": - - // Default values - awsAccessID := "changeme" - awsAccessKey := "changeme" - awsSessionToken := "" - awsRegion := "changeme" - awsAccount := "changeme" - awsRole := "changeme" - gcrApplicationDefaultCredentials := "changeme" - dockerServer := "changeme" - dockerUser := "changeme" - dockerPass := "changeme" - gcrURL := "https://gcr.io" - acrURL := "changeme" - acrClientID := "changeme" - acrPassword := "changeme" - - enableAWSECR := AskForYesNoConfirmation("\nDo you want to enable AWS Elastic Container Registry?", posResponses, negResponses) - if enableAWSECR { - awsAccessID = AskForStaticValue("-- Enter AWS Access Key ID: ") - awsAccessKey = AskForStaticValue("-- Enter AWS Secret Access Key: ") - awsSessionToken = AskForStaticValueOptional("-- (Optional) Enter AWS Session Token: ") - awsRegion = AskForStaticValue("-- Enter AWS Region: ") - awsAccount = AskForStaticValue("-- Enter 12 digit AWS Account ID (Comma separated list): ") - awsRole = AskForStaticValueOptional("-- (Optional) Enter ARN of AWS role to assume: ") - } - - enableGCR := AskForYesNoConfirmation("\nDo you want to enable Google Container Registry?", posResponses, negResponses) - if enableGCR { - gcrPath := AskForStaticValue("-- Enter path to credentials (e.g. /home/user/.config/gcloud/application_default_credentials.json):") - gcrchangeURL := AskForYesNoConfirmation("-- Do you want to change the GCR URL (Default https://gcr.io)?", posResponses, negResponses) - - if gcrchangeURL { - gcrURL = AskForStaticValue("-- Enter GCR URL (e.g. https://asia.gcr.io):") - } - - // Read file from disk - dat, err := os.ReadFile(gcrPath) - - if err != nil { - out.FailureT("Error reading {{.path}}: {{.error}}", out.V{"path": gcrPath, "error": err}) - } else { - gcrApplicationDefaultCredentials = string(dat) - } - } - - enableDR := AskForYesNoConfirmation("\nDo you want to enable Docker Registry?", posResponses, negResponses) - if enableDR { - dockerServer = AskForStaticValue("-- Enter docker registry server url: ") - dockerUser = AskForStaticValue("-- Enter docker registry username: ") - dockerPass = AskForPasswordValue("-- Enter docker registry password: ") - } - - enableACR := AskForYesNoConfirmation("\nDo you want to enable Azure Container Registry?", posResponses, negResponses) - if enableACR { - acrURL = AskForStaticValue("-- Enter Azure Container Registry (ACR) URL: ") - acrClientID = AskForStaticValue("-- Enter client ID (service principal ID) to access ACR: ") - acrPassword = AskForPasswordValue("-- Enter service principal password to access Azure Container Registry: ") - } - - namespace := "kube-system" - - // Create ECR Secret - err := service.CreateSecret( - profile, - namespace, - "registry-creds-ecr", - map[string]string{ - "AWS_ACCESS_KEY_ID": awsAccessID, - "AWS_SECRET_ACCESS_KEY": awsAccessKey, - "AWS_SESSION_TOKEN": awsSessionToken, - "aws-account": awsAccount, - "aws-region": awsRegion, - "aws-assume-role": awsRole, - }, - map[string]string{ - "app": "registry-creds", - "cloud": "ecr", - "kubernetes.io/minikube-addons": "registry-creds", - }) - if err != nil { - out.FailureT("ERROR creating `registry-creds-ecr` secret: {{.error}}", out.V{"error": err}) - } - - // Create GCR Secret - err = service.CreateSecret( - profile, - namespace, - "registry-creds-gcr", - map[string]string{ - "application_default_credentials.json": gcrApplicationDefaultCredentials, - "gcrurl": gcrURL, - }, - map[string]string{ - "app": "registry-creds", - "cloud": "gcr", - "kubernetes.io/minikube-addons": "registry-creds", - }) - - if err != nil { - out.FailureT("ERROR creating `registry-creds-gcr` secret: {{.error}}", out.V{"error": err}) - } - - // Create Docker Secret - err = service.CreateSecret( - profile, - namespace, - "registry-creds-dpr", - map[string]string{ - "DOCKER_PRIVATE_REGISTRY_SERVER": dockerServer, - "DOCKER_PRIVATE_REGISTRY_USER": dockerUser, - "DOCKER_PRIVATE_REGISTRY_PASSWORD": dockerPass, - }, - map[string]string{ - "app": "registry-creds", - "cloud": "dpr", - "kubernetes.io/minikube-addons": "registry-creds", - }) - - if err != nil { - out.WarningT("ERROR creating `registry-creds-dpr` secret") - } - - // Create Azure Container Registry Secret - err = service.CreateSecret( - profile, - namespace, - "registry-creds-acr", - map[string]string{ - "ACR_URL": acrURL, - "ACR_CLIENT_ID": acrClientID, - "ACR_PASSWORD": acrPassword, - }, - map[string]string{ - "app": "registry-creds", - "cloud": "acr", - "kubernetes.io/minikube-addons": "registry-creds", - }) - - if err != nil { - out.WarningT("ERROR creating `registry-creds-acr` secret") - } + processRegistryCredsConfig(profile, addonConfig) case "metallb": - _, cfg := mustload.Partial(profile) - - validator := func(s string) bool { - return net.ParseIP(s) != nil - } - - cfg.KubernetesConfig.LoadBalancerStartIP = AskForStaticValidatedValue("-- Enter Load Balancer Start IP: ", validator) - - cfg.KubernetesConfig.LoadBalancerEndIP = AskForStaticValidatedValue("-- Enter Load Balancer End IP: ", validator) - - if err := config.SaveProfile(profile, cfg); err != nil { - out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) - } + processMetalLBConfig(profile, addonConfig) - // Re-enable metallb addon in order to generate template manifest files with Load Balancer Start/End IP - if err := addons.EnableOrDisableAddon(cfg, "metallb", "true"); err != nil { - out.ErrT(style.Fatal, "Failed to configure metallb IP {{.profile}}", out.V{"profile": profile}) - } case "ingress": - _, cfg := mustload.Partial(profile) - - validator := func(s string) bool { - format := regexp.MustCompile("^.+/.+$") - return format.MatchString(s) - } + processIngressConfig(profile, addonConfig) - customCert := AskForStaticValidatedValue("-- Enter custom cert (format is \"namespace/secret\"): ", validator) - if cfg.KubernetesConfig.CustomIngressCert != "" { - overwrite := AskForYesNoConfirmation("A custom cert for ingress has already been set. Do you want overwrite it?", posResponses, negResponses) - if !overwrite { - break - } - } - - cfg.KubernetesConfig.CustomIngressCert = customCert - - if err := config.SaveProfile(profile, cfg); err != nil { - out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) - } case "registry-aliases": - _, cfg := mustload.Partial(profile) - validator := func(s string) bool { - format := regexp.MustCompile(`^([a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)+(\ [a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)*$`) - return format.MatchString(s) - } - registryAliases := AskForStaticValidatedValue("-- Enter registry aliases separated by space: ", validator) - cfg.KubernetesConfig.RegistryAliases = registryAliases + processRegistryAliasesConfig(profile, addonConfig) - if err := config.SaveProfile(profile, cfg); err != nil { - out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) - } - addon := assets.Addons["registry-aliases"] - if addon.IsEnabled(cfg) { - // Re-enable registry-aliases addon in order to generate template manifest files with custom hosts - if err := addons.EnableOrDisableAddon(cfg, "registry-aliases", "true"); err != nil { - out.ErrT(style.Fatal, "Failed to configure registry-aliases {{.profile}}", out.V{"profile": profile}) - } - } case "auto-pause": - lapi, cfg := mustload.Partial(profile) - intervalInput := AskForStaticValue("-- Enter interval time of auto-pause-interval (ex. 1m0s): ") - intervalTime, err := time.ParseDuration(intervalInput) - if err != nil { - out.ErrT(style.Fatal, "Interval is an invalid duration: {{.error}}", out.V{"error": err}) - } - if intervalTime != intervalTime.Abs() || intervalTime.String() == "0s" { - out.ErrT(style.Fatal, "Interval must be greater than 0s") - } - cfg.AutoPauseInterval = intervalTime - if err := config.SaveProfile(profile, cfg); err != nil { - out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) - } - addon := assets.Addons["auto-pause"] - if addon.IsEnabled(cfg) { + processAutoPauseConfig(profile, addonConfig) - // see #17945: restart auto-pause service - p, err := config.LoadProfile(profile) - if err != nil { - out.ErrT(style.Fatal, "failed to load profile: {{.error}}", out.V{"error": err}) - } - if profileStatus(p, lapi).StatusCode/100 == 2 { // 2xx code - co := mustload.Running(profile) - // first unpause all nodes cluster immediately - unpauseWholeCluster(co) - // Re-enable auto-pause addon in order to update interval time - if err := addons.EnableOrDisableAddon(cfg, "auto-pause", "true"); err != nil { - out.ErrT(style.Fatal, "Failed to configure auto-pause {{.profile}}", out.V{"profile": profile}) - } - // restart auto-pause service - if err := sysinit.New(co.CP.Runner).Restart("auto-pause"); err != nil { - out.ErrT(style.Fatal, "failed to restart auto-pause: {{.error}}", out.V{"error": err}) - } - - } - } default: out.FailureT("{{.name}} has no available configuration options", out.V{"name": addon}) return @@ -339,5 +123,375 @@ func unpauseWholeCluster(co mustload.ClusterController) { } func init() { + addonsConfigureCmd.Flags().StringVarP(&AddonConfigFile, "config-file", "f", "", "An optional configuration file to read addon specific configs from instead of being prompted each time.") AddonsCmd.AddCommand(addonsConfigureCmd) } + +// Helper method to load a config file for addons +func loadAddonConfigFile(addon, configFilePath string) (addonConfig map[string]any, err error) { + if configFilePath != "" { + configFileData := make(map[string]any) + out.Ln("Reading %s configs from %s", addon, configFilePath) + if confData, err := os.ReadFile(configFilePath); err != nil && errors.Is(err, os.ErrNotExist) { + exit.Message(reason.Usage, "config file does not exist") + return nil, err + } else if err != nil { + exit.Message(reason.Kind{ExitCode: reason.ExProgramConfig, Advice: "provide a valid config file"}, + fmt.Sprintf("error opening config file: %v", err)) + return nil, err + } else if err = json.Unmarshal(confData, &configFileData); err != nil { + exit.Message(reason.Kind{ExitCode: reason.ExProgramConfig, Advice: "provide a valid config file"}, + fmt.Sprintf("error opening config file: %v", err)) + return nil, err + } + + // Make sure the addon specific config exists and it is a map + if addonSection, ok := configFileData["addons"]; ok && addonSection != nil { + if addonSectionMap, ok := addonSection.(map[string]any); ok && addonSectionMap != nil { + if addonSpecificConfig, ok := addonSectionMap[addon]; ok && addonSpecificConfig != nil { + if casted, ok := addonSpecificConfig.(map[string]any); casted != nil && ok { + addonConfig = casted + } + } + } + } + } + return +} + +// Given a map, returns the (string) value of a key in a given path, equivalent of a["x]["y"]["z"]. +// In case of errors or type mismatches (eg missing key paths or invalid types) an empty string is returned. +func getNestedJSONString(configMap map[string]any, keypath ...string) string { + for idx, key := range keypath { + next, ok := configMap[key] + if !ok || next == nil { + break + } + if idx == len(keypath)-1 { + strval, ok := next.(string) + + if ok { + return strval + } + out.Ln("Expected string at last key, found: ", reflect.TypeOf(next), next) + } else { + if mapval, ok := next.(map[string]any); ok && mapval != nil { + configMap = mapval + } else { + out.Stringf("expected map[string]any at %d, found: %v", idx, mapval) + break + } + } + } + return "" +} + +// Processes metallb addon config from configFile if it exists otherwise resorts to default behavior +func processMetalLBConfig(profile string, _ map[string]any) { + _, cfg := mustload.Partial(profile) + + validator := func(s string) bool { + return net.ParseIP(s) != nil + } + + cfg.KubernetesConfig.LoadBalancerStartIP = AskForStaticValidatedValue("-- Enter Load Balancer Start IP: ", validator) + + cfg.KubernetesConfig.LoadBalancerEndIP = AskForStaticValidatedValue("-- Enter Load Balancer End IP: ", validator) + + if err := config.SaveProfile(profile, cfg); err != nil { + out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) + } + + // Re-enable metallb addon in order to generate template manifest files with Load Balancer Start/End IP + if err := addons.EnableOrDisableAddon(cfg, "metallb", "true"); err != nil { + out.ErrT(style.Fatal, "Failed to configure metallb IP {{.profile}}", out.V{"profile": profile}) + } +} + +// Processes ingress addon config from configFile if it exists otherwise resorts to default behavior +func processIngressConfig(profile string, _ map[string]any) { + _, cfg := mustload.Partial(profile) + + validator := func(s string) bool { + format := regexp.MustCompile("^.+/.+$") + return format.MatchString(s) + } + + customCert := AskForStaticValidatedValue("-- Enter custom cert (format is \"namespace/secret\"): ", validator) + if cfg.KubernetesConfig.CustomIngressCert != "" { + overwrite := AskForYesNoConfirmation("A custom cert for ingress has already been set. Do you want overwrite it?", posResponses, negResponses) + if !overwrite { + return + } + } + + cfg.KubernetesConfig.CustomIngressCert = customCert + + if err := config.SaveProfile(profile, cfg); err != nil { + out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) + } +} + +// Processes auto-pause addon config from configFile if it exists otherwise resorts to default behavior +func processAutoPauseConfig(profile string, _ map[string]any) { + lapi, cfg := mustload.Partial(profile) + intervalInput := AskForStaticValue("-- Enter interval time of auto-pause-interval (ex. 1m0s): ") + intervalTime, err := time.ParseDuration(intervalInput) + if err != nil { + out.ErrT(style.Fatal, "Interval is an invalid duration: {{.error}}", out.V{"error": err}) + } + + if intervalTime != intervalTime.Abs() || intervalTime.String() == "0s" { + out.ErrT(style.Fatal, "Interval must be greater than 0s") + } + + cfg.AutoPauseInterval = intervalTime + if err = config.SaveProfile(profile, cfg); err != nil { + out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) + } + + addon := assets.Addons["auto-pause"] + if addon.IsEnabled(cfg) { + + // see #17945: restart auto-pause service + p, err := config.LoadProfile(profile) + if err != nil { + out.ErrT(style.Fatal, "failed to load profile: {{.error}}", out.V{"error": err}) + } + if profileStatus(p, lapi).StatusCode/100 == 2 { // 2xx code + co := mustload.Running(profile) + // first unpause all nodes cluster immediately + unpauseWholeCluster(co) + // Re-enable auto-pause addon in order to update interval time + if err := addons.EnableOrDisableAddon(cfg, "auto-pause", "true"); err != nil { + out.ErrT(style.Fatal, "Failed to configure auto-pause {{.profile}}", out.V{"profile": profile}) + } + // restart auto-pause service + if err := sysinit.New(co.CP.Runner).Restart("auto-pause"); err != nil { + out.ErrT(style.Fatal, "failed to restart auto-pause: {{.error}}", out.V{"error": err}) + } + } + } +} + +// Processes registry-aliases addon config from configFile if it exists otherwise resorts to default behavior +func processRegistryAliasesConfig(profile string, _ map[string]any) { + _, cfg := mustload.Partial(profile) + validator := func(s string) bool { + format := regexp.MustCompile(`^([a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)+(\ [a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)*$`) + return format.MatchString(s) + } + registryAliases := AskForStaticValidatedValue("-- Enter registry aliases separated by space: ", validator) + cfg.KubernetesConfig.RegistryAliases = registryAliases + + if err := config.SaveProfile(profile, cfg); err != nil { + out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile}) + } + + addon := assets.Addons["registry-aliases"] + if addon.IsEnabled(cfg) { + // Re-enable registry-aliases addon in order to generate template manifest files with custom hosts + if err := addons.EnableOrDisableAddon(cfg, "registry-aliases", "true"); err != nil { + out.ErrT(style.Fatal, "Failed to configure registry-aliases {{.profile}}", out.V{"profile": profile}) + } + } +} + +// Processes registry-creds addon config from configFile if it exists otherwise resorts to default behavior +func processRegistryCredsConfig(profile string, configFileData map[string]any) { + // Default values + awsAccessID := "changeme" + awsAccessKey := "changeme" + awsSessionToken := "" + awsRegion := "changeme" + awsAccount := "changeme" + awsRole := "changeme" + gcrApplicationDefaultCredentials := "changeme" + dockerServer := "changeme" + dockerUser := "changeme" + dockerPass := "changeme" + gcrURL := "https://gcr.io" + acrURL := "changeme" + acrClientID := "changeme" + acrPassword := "changeme" + + awsEcrAction := getNestedJSONString(configFileData, "enableAWSEcr") + if awsEcrAction == "prompt" || awsEcrAction == "" { + enableAWSECR := AskForYesNoConfirmation("\nDo you want to enable AWS Elastic Container Registry?", posResponses, negResponses) + if enableAWSECR { + awsAccessID = AskForStaticValue("-- Enter AWS Access Key ID: ") + awsAccessKey = AskForStaticValue("-- Enter AWS Secret Access Key: ") + awsSessionToken = AskForStaticValueOptional("-- (Optional) Enter AWS Session Token: ") + awsRegion = AskForStaticValue("-- Enter AWS Region: ") + awsAccount = AskForStaticValue("-- Enter 12 digit AWS Account ID (Comma separated list): ") + awsRole = AskForStaticValueOptional("-- (Optional) Enter ARN of AWS role to assume: ") + } + } else if awsEcrAction == "enable" { + out.Ln("Loading AWS ECR configs from: %s", AddonConfigFile) + // Then read the configs + awsAccessID = getNestedJSONString(configFileData, "awsEcrConfigs", "awsAccessID") + awsAccessKey = getNestedJSONString(configFileData, "awsEcrConfigs", "awsAccessKey") + awsSessionToken = getNestedJSONString(configFileData, "awsEcrConfigs", "awsSessionToken") + awsRegion = getNestedJSONString(configFileData, "awsEcrConfigs", "awsRegion") + awsAccount = getNestedJSONString(configFileData, "awsEcrConfigs", "awsAccount") + awsRole = getNestedJSONString(configFileData, "awsEcrConfigs", "awsRole") + } else if awsEcrAction == "disable" { + out.Ln("Ignoring AWS ECR configs") + } else { + out.Ln("Disabling AWS ECR. Invalid value for enableAWSEcr (%s). Must be one of 'disable', 'enable' or 'prompt'", awsEcrAction) + } + + gcrPath := "" + gcrAction := getNestedJSONString(configFileData, "enableGCR") + if gcrAction == "prompt" || gcrAction == "" { + enableGCR := AskForYesNoConfirmation("\nDo you want to enable Google Container Registry?", posResponses, negResponses) + if enableGCR { + gcrPath = AskForStaticValue("-- Enter path to credentials (e.g. /home/user/.config/gcloud/application_default_credentials.json):") + gcrchangeURL := AskForYesNoConfirmation("-- Do you want to change the GCR URL (Default https://gcr.io)?", posResponses, negResponses) + + if gcrchangeURL { + gcrURL = AskForStaticValue("-- Enter GCR URL (e.g. https://asia.gcr.io):") + } + } + } else if gcrAction == "enable" { + out.Ln("Loading GCR configs from: %s", AddonConfigFile) + // Then read the configs + gcrPath = getNestedJSONString(configFileData, "gcrConfigs", "gcrPath") + gcrURL = getNestedJSONString(configFileData, "gcrConfigs", "gcrURL") + } else if gcrAction == "disable" { + out.Ln("Ignoring GCR configs") + } else { + out.Ln("Disabling GCR. Invalid value for enableGCR (%s). Must be one of 'disable', 'enable' or 'prompt'", gcrAction) + } + + if gcrPath != "" { + // Read file from disk + dat, err := os.ReadFile(gcrPath) + + if err != nil { + exit.Message(reason.Usage, "Error reading {{.path}}: {{.error}}", out.V{"path": gcrPath, "error": err}) + } else { + gcrApplicationDefaultCredentials = string(dat) + } + } + + dockerRegistryAction := getNestedJSONString(configFileData, "enableDockerRegistry") + if dockerRegistryAction == "prompt" || dockerRegistryAction == "" { + enableDR := AskForYesNoConfirmation("\nDo you want to enable Docker Registry?", posResponses, negResponses) + if enableDR { + dockerServer = AskForStaticValue("-- Enter docker registry server url: ") + dockerUser = AskForStaticValue("-- Enter docker registry username: ") + dockerPass = AskForPasswordValue("-- Enter docker registry password: ") + } + } else if dockerRegistryAction == "enable" { + out.Ln("Loading Docker Registry configs from: %s", AddonConfigFile) + dockerServer = getNestedJSONString(configFileData, "dockerConfigs", "dockerServer") + dockerUser = getNestedJSONString(configFileData, "dockerConfigs", "dockerUser") + dockerPass = getNestedJSONString(configFileData, "dockerConfigs", "dockerPass") + } else if dockerRegistryAction == "disable" { + out.Ln("Ignoring Docker Registry configs") + } else { + out.Ln("Disabling Docker Registry. Invalid value for enableDockerRegistry (%s). Must be one of 'disable', 'enable' or 'prompt'", dockerRegistryAction) + } + + acrAction := getNestedJSONString(configFileData, "enableACR") + if acrAction == "prompt" || acrAction == "" { + enableACR := AskForYesNoConfirmation("\nDo you want to enable Azure Container Registry?", posResponses, negResponses) + if enableACR { + acrURL = AskForStaticValue("-- Enter Azure Container Registry (ACR) URL: ") + acrClientID = AskForStaticValue("-- Enter client ID (service principal ID) to access ACR: ") + acrPassword = AskForPasswordValue("-- Enter service principal password to access Azure Container Registry: ") + } + } else if configFileData == nil || acrAction == "enable" { + out.Ln("Loading ACR configs from: ", AddonConfigFile) + acrURL = getNestedJSONString(configFileData, "acrConfigs", "acrURL") + acrClientID = getNestedJSONString(configFileData, "acrConfigs", "acrClientID") + acrPassword = getNestedJSONString(configFileData, "acrConfigs", "acrPassword") + } else if acrAction == "disable" { + out.Ln("Ignoring ACR configs") + } else { + out.Stringf("Disabling ACR. Invalid value for enableACR (%s). Must be one of 'disable', 'enable' or 'prompt'", configFileData["enableACR"]) + } + + namespace := "kube-system" + + // Create ECR Secret + err := service.CreateSecret( + profile, + namespace, + "registry-creds-ecr", + map[string]string{ + "AWS_ACCESS_KEY_ID": awsAccessID, + "AWS_SECRET_ACCESS_KEY": awsAccessKey, + "AWS_SESSION_TOKEN": awsSessionToken, + "aws-account": awsAccount, + "aws-region": awsRegion, + "aws-assume-role": awsRole, + }, + map[string]string{ + "app": "registry-creds", + "cloud": "ecr", + "kubernetes.io/minikube-addons": "registry-creds", + }) + if err != nil { + exit.Message(reason.InternalCommandRunner, "ERROR creating `registry-creds-ecr` secret: {{.error}}", out.V{"error": err}) + } + + // Create GCR Secret + err = service.CreateSecret( + profile, + namespace, + "registry-creds-gcr", + map[string]string{ + "application_default_credentials.json": gcrApplicationDefaultCredentials, + "gcrurl": gcrURL, + }, + map[string]string{ + "app": "registry-creds", + "cloud": "gcr", + "kubernetes.io/minikube-addons": "registry-creds", + }) + + if err != nil { + exit.Message(reason.InternalCommandRunner, "ERROR creating `registry-creds-gcr` secret: {{.error}}", out.V{"error": err}) + } + + // Create Docker Secret + err = service.CreateSecret( + profile, + namespace, + "registry-creds-dpr", + map[string]string{ + "DOCKER_PRIVATE_REGISTRY_SERVER": dockerServer, + "DOCKER_PRIVATE_REGISTRY_USER": dockerUser, + "DOCKER_PRIVATE_REGISTRY_PASSWORD": dockerPass, + }, + map[string]string{ + "app": "registry-creds", + "cloud": "dpr", + "kubernetes.io/minikube-addons": "registry-creds", + }) + + if err != nil { + out.WarningT("ERROR creating `registry-creds-dpr` secret") + } + + // Create Azure Container Registry Secret + err = service.CreateSecret( + profile, + namespace, + "registry-creds-acr", + map[string]string{ + "ACR_URL": acrURL, + "ACR_CLIENT_ID": acrClientID, + "ACR_PASSWORD": acrPassword, + }, + map[string]string{ + "app": "registry-creds", + "cloud": "acr", + "kubernetes.io/minikube-addons": "registry-creds", + }) + if err != nil { + out.WarningT("ERROR creating `registry-creds-acr` secret") + } +} diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index f060af61d3c4..d3d3b79b53a0 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -100,7 +100,7 @@ func TestAddons(t *testing.T) { // so we override that here to let minikube auto-detect appropriate cgroup driver os.Setenv(constants.MinikubeForceSystemdEnv, "") - args := append([]string{"start", "-p", profile, "--wait=true", "--memory=4000", "--alsologtostderr", "--addons=registry", "--addons=metrics-server", "--addons=volumesnapshots", "--addons=csi-hostpath-driver", "--addons=gcp-auth", "--addons=cloud-spanner", "--addons=inspektor-gadget", "--addons=nvidia-device-plugin", "--addons=yakd", "--addons=volcano", "--addons=amd-gpu-device-plugin"}, StartArgs()...) + args := append([]string{"start", "-p", profile, "--wait=true", "--memory=4000", "--alsologtostderr", "--addons=registry", "--addons=registry-creds", "--addons=metrics-server", "--addons=volumesnapshots", "--addons=csi-hostpath-driver", "--addons=gcp-auth", "--addons=cloud-spanner", "--addons=inspektor-gadget", "--addons=nvidia-device-plugin", "--addons=yakd", "--addons=volcano", "--addons=amd-gpu-device-plugin"}, StartArgs()...) if !NoneDriver() { args = append(args, "--addons=ingress", "--addons=ingress-dns", "--addons=storage-provisioner-rancher") } @@ -140,6 +140,7 @@ func TestAddons(t *testing.T) { t.Run("parallel", func(t *testing.T) { tests := []TestCase{ {"Registry", validateRegistryAddon}, + {"RegistryCreds", validateRegistryCredsAddon}, {"Ingress", validateIngressAddon}, {"InspektorGadget", validateInspektorGadgetAddon}, {"MetricsServer", validateMetricsServerAddon}, @@ -304,6 +305,64 @@ func validateIngressAddon(ctx context.Context, t *testing.T, profile string) { } } +// validateRegistryCredsAddon tests the registry-creds addon by trying to load its configs +func validateRegistryCredsAddon(ctx context.Context, t *testing.T, profile string) { + defer disableAddon(t, "registry-creds", profile) + defer PostMortemLogs(t, profile) + + client, err := kapi.Client(profile) + if err != nil { + t.Fatalf("failed to get Kubernetes client for %s : %v", profile, err) + } + + start := time.Now() + if err := kapi.WaitForDeploymentToStabilize(client, "kube-system", "registry-creds", Minutes(6)); err != nil { + t.Errorf("failed waiting for registry-creds deployment to stabilize: %v", err) + } + t.Logf("registry-creds stabilized in %s", time.Since(start)) + + rr, err := Run(t, exec.CommandContext(ctx, Target(), "addons", "configure", "registry-creds", "-f", "./testdata/addons_testconfig.json", "-p", profile)) + if err != nil { + t.Errorf("failed to configure addon. args %q : %v", rr.Command(), err) + } + + // Check a few secrets exists that match our test data + // In our test aws and gcp are set, docker and acr are disabled - so they will be set to "changeme" + rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "-n", "kube-system", "get", "secret", "-o", "yaml")) + if err != nil { + t.Errorf("failed to get secrets. args %q : %v", rr.Command(), err) + } + + expected := []string{ + "DOCKER_PRIVATE_REGISTRY_PASSWORD: Y2hhbmdlbWU=", + "DOCKER_PRIVATE_REGISTRY_SERVER: Y2hhbmdlbWU=", + "DOCKER_PRIVATE_REGISTRY_USER: Y2hhbmdlbWU=", + + "ACR_CLIENT_ID: Y2hhbmdlbWU=", + "ACR_PASSWORD: Y2hhbmdlbWU=", + "ACR_URL: Y2hhbmdlbWU=", + + "AWS_ACCESS_KEY_ID: dGVzdF9hd3NfYWNjZXNzaWQ=", + "AWS_SECRET_ACCESS_KEY: dGVzdF9hd3NfYWNjZXNza2V5", + "AWS_SESSION_TOKEN: dGVzdF9hd3Nfc2Vzc2lvbl90b2tlbg==", + "aws-account: dGVzdF9hd3NfYWNjb3VudA==", + "aws-assume-role: dGVzdF9hd3Nfcm9sZQ==", + "aws-region: dGVzdF9hd3NfcmVnaW9u", + + "application_default_credentials.json: ewogICJjbGllbnRfaWQiOiAiaGFoYSIsCiAgImNsaWVudF9zZWNyZXQiOiAibmljZV90cnkiLAogICJxdW90YV9wcm9qZWN0X2lkIjogInRoaXNfaXNfZmFrZSIsCiAgInJlZnJlc2hfdG9rZW4iOiAibWF5YmVfbmV4dF90aW1lIiwKICAidHlwZSI6ICJhdXRob3JpemVkX3VzZXIiCn0K", + "gcrurl: aHR0cHM6Ly9nY3IuaW8=", + } + + rrout := strings.TrimSpace(rr.Stdout.String()) + for _, exp := range expected { + re := regexp.MustCompile(fmt.Sprintf(".*%s.*", exp)) + secret := re.FindString(rrout) + if secret == "" { + t.Errorf("Did not find expected secret: '%s'", secret) + } + } +} + // validateRegistryAddon tests the registry addon func validateRegistryAddon(ctx context.Context, t *testing.T, profile string) { defer disableAddon(t, "registry", profile) diff --git a/test/integration/testdata/addons_testconfig.json b/test/integration/testdata/addons_testconfig.json new file mode 100644 index 000000000000..3805b5737b2b --- /dev/null +++ b/test/integration/testdata/addons_testconfig.json @@ -0,0 +1,32 @@ + { + "addons": { + "registry-creds": { + "enableAWSEcr": "enable", + "awsEcrConfigs": { + "awsAccessID": "test_aws_accessid", + "awsAccessKey": "test_aws_accesskey", + "awsSessionToken": "test_aws_session_token", + "awsRegion": "test_aws_region", + "awsAccount": "test_aws_account", + "awsRole": "test_aws_role" + }, + "enableGCR": "enable", + "gcrConfigs": { + "gcrPath": "./testdata/gcp-creds.json", + "gcrURL": "https://gcr.io" + }, + "enableDockerRegistry": "disable", + "dockerConfigs": { + "dockerServer": "test_docker_server", + "dockerUser": "test_docker_user", + "dockerPass": "test_docker_password" + }, + "enableACR": "disable", + "acrConfigs": { + "acrURL": "test_acr_url", + "acrClientID": "test_acr_clientid", + "acrPassword": "test_acr_password" + } + } + } + } From 5cbc860b6beb4c97fa4a27b0b548bb4841dc713a Mon Sep 17 00:00:00 2001 From: Sriram Panyam Date: Fri, 17 Jan 2025 11:43:03 -0800 Subject: [PATCH 2/4] Using a rarer value for default instead of changeme so it has less chance to interfere with other possible user defaults --- cmd/minikube/cmd/config/configure.go | 24 ++++++++++++------------ test/integration/addons_test.go | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cmd/minikube/cmd/config/configure.go b/cmd/minikube/cmd/config/configure.go index d5039df5e844..728421a9b7ff 100644 --- a/cmd/minikube/cmd/config/configure.go +++ b/cmd/minikube/cmd/config/configure.go @@ -300,20 +300,20 @@ func processRegistryAliasesConfig(profile string, _ map[string]any) { // Processes registry-creds addon config from configFile if it exists otherwise resorts to default behavior func processRegistryCredsConfig(profile string, configFileData map[string]any) { // Default values - awsAccessID := "changeme" - awsAccessKey := "changeme" + awsAccessID := "MINIKUBE_DEFAULT_VALUE" + awsAccessKey := "MINIKUBE_DEFAULT_VALUE" awsSessionToken := "" - awsRegion := "changeme" - awsAccount := "changeme" - awsRole := "changeme" - gcrApplicationDefaultCredentials := "changeme" - dockerServer := "changeme" - dockerUser := "changeme" - dockerPass := "changeme" + awsRegion := "MINIKUBE_DEFAULT_VALUE" + awsAccount := "MINIKUBE_DEFAULT_VALUE" + awsRole := "MINIKUBE_DEFAULT_VALUE" + gcrApplicationDefaultCredentials := "MINIKUBE_DEFAULT_VALUE" + dockerServer := "MINIKUBE_DEFAULT_VALUE" + dockerUser := "MINIKUBE_DEFAULT_VALUE" + dockerPass := "MINIKUBE_DEFAULT_VALUE" gcrURL := "https://gcr.io" - acrURL := "changeme" - acrClientID := "changeme" - acrPassword := "changeme" + acrURL := "MINIKUBE_DEFAULT_VALUE" + acrClientID := "MINIKUBE_DEFAULT_VALUE" + acrPassword := "MINIKUBE_DEFAULT_VALUE" awsEcrAction := getNestedJSONString(configFileData, "enableAWSEcr") if awsEcrAction == "prompt" || awsEcrAction == "" { diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index d3d3b79b53a0..b5379f2ab795 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -327,7 +327,7 @@ func validateRegistryCredsAddon(ctx context.Context, t *testing.T, profile strin } // Check a few secrets exists that match our test data - // In our test aws and gcp are set, docker and acr are disabled - so they will be set to "changeme" + // In our test aws and gcp are set, docker and acr are disabled - so they will be set to "MINIKUBE_DEFAULT_VALUE" rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "-n", "kube-system", "get", "secret", "-o", "yaml")) if err != nil { t.Errorf("failed to get secrets. args %q : %v", rr.Command(), err) From 1485c750b14d89817fcefc1d3d736b15008c0b31 Mon Sep 17 00:00:00 2001 From: Sriram Panyam Date: Mon, 20 Jan 2025 21:57:57 -0800 Subject: [PATCH 3/4] Fixing expected values of test cases --- test/integration/addons_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index b5379f2ab795..64aec570df3b 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -334,13 +334,13 @@ func validateRegistryCredsAddon(ctx context.Context, t *testing.T, profile strin } expected := []string{ - "DOCKER_PRIVATE_REGISTRY_PASSWORD: Y2hhbmdlbWU=", - "DOCKER_PRIVATE_REGISTRY_SERVER: Y2hhbmdlbWU=", - "DOCKER_PRIVATE_REGISTRY_USER: Y2hhbmdlbWU=", + "DOCKER_PRIVATE_REGISTRY_PASSWORD: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", + "DOCKER_PRIVATE_REGISTRY_SERVER: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", + "DOCKER_PRIVATE_REGISTRY_USER: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", - "ACR_CLIENT_ID: Y2hhbmdlbWU=", - "ACR_PASSWORD: Y2hhbmdlbWU=", - "ACR_URL: Y2hhbmdlbWU=", + "ACR_CLIENT_ID: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", + "ACR_PASSWORD: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", + "ACR_URL: TUlOSUtVQkVfREVGQVVMVF9WQUxVRQ==", "AWS_ACCESS_KEY_ID: dGVzdF9hd3NfYWNjZXNzaWQ=", "AWS_SECRET_ACCESS_KEY: dGVzdF9hd3NfYWNjZXNza2V5", From f5776e767dbc62396ad15c8d2e3fd78f9872eac5 Mon Sep 17 00:00:00 2001 From: Sriram Panyam Date: Wed, 22 Jan 2025 11:24:02 -0800 Subject: [PATCH 4/4] making addonConfigFile private --- cmd/minikube/cmd/config/configure.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/minikube/cmd/config/configure.go b/cmd/minikube/cmd/config/configure.go index 728421a9b7ff..ad5d6eb41176 100644 --- a/cmd/minikube/cmd/config/configure.go +++ b/cmd/minikube/cmd/config/configure.go @@ -42,7 +42,7 @@ import ( "k8s.io/minikube/pkg/minikube/sysinit" ) -var AddonConfigFile = "" +var addonConfigFile = "" var posResponses = []string{"yes", "y"} var negResponses = []string{"no", "n"} @@ -57,7 +57,7 @@ var addonsConfigureCmd = &cobra.Command{ profile := ClusterFlagValue() addon := args[0] - addonConfig, err := loadAddonConfigFile(addon, AddonConfigFile) + addonConfig, err := loadAddonConfigFile(addon, addonConfigFile) if err != nil { return } @@ -123,7 +123,7 @@ func unpauseWholeCluster(co mustload.ClusterController) { } func init() { - addonsConfigureCmd.Flags().StringVarP(&AddonConfigFile, "config-file", "f", "", "An optional configuration file to read addon specific configs from instead of being prompted each time.") + addonsConfigureCmd.Flags().StringVarP(&addonConfigFile, "config-file", "f", "", "An optional configuration file to read addon specific configs from instead of being prompted each time.") AddonsCmd.AddCommand(addonsConfigureCmd) } @@ -327,7 +327,7 @@ func processRegistryCredsConfig(profile string, configFileData map[string]any) { awsRole = AskForStaticValueOptional("-- (Optional) Enter ARN of AWS role to assume: ") } } else if awsEcrAction == "enable" { - out.Ln("Loading AWS ECR configs from: %s", AddonConfigFile) + out.Ln("Loading AWS ECR configs from: %s", addonConfigFile) // Then read the configs awsAccessID = getNestedJSONString(configFileData, "awsEcrConfigs", "awsAccessID") awsAccessKey = getNestedJSONString(configFileData, "awsEcrConfigs", "awsAccessKey") @@ -354,7 +354,7 @@ func processRegistryCredsConfig(profile string, configFileData map[string]any) { } } } else if gcrAction == "enable" { - out.Ln("Loading GCR configs from: %s", AddonConfigFile) + out.Ln("Loading GCR configs from: %s", addonConfigFile) // Then read the configs gcrPath = getNestedJSONString(configFileData, "gcrConfigs", "gcrPath") gcrURL = getNestedJSONString(configFileData, "gcrConfigs", "gcrURL") @@ -384,7 +384,7 @@ func processRegistryCredsConfig(profile string, configFileData map[string]any) { dockerPass = AskForPasswordValue("-- Enter docker registry password: ") } } else if dockerRegistryAction == "enable" { - out.Ln("Loading Docker Registry configs from: %s", AddonConfigFile) + out.Ln("Loading Docker Registry configs from: %s", addonConfigFile) dockerServer = getNestedJSONString(configFileData, "dockerConfigs", "dockerServer") dockerUser = getNestedJSONString(configFileData, "dockerConfigs", "dockerUser") dockerPass = getNestedJSONString(configFileData, "dockerConfigs", "dockerPass") @@ -403,7 +403,7 @@ func processRegistryCredsConfig(profile string, configFileData map[string]any) { acrPassword = AskForPasswordValue("-- Enter service principal password to access Azure Container Registry: ") } } else if configFileData == nil || acrAction == "enable" { - out.Ln("Loading ACR configs from: ", AddonConfigFile) + out.Ln("Loading ACR configs from: ", addonConfigFile) acrURL = getNestedJSONString(configFileData, "acrConfigs", "acrURL") acrClientID = getNestedJSONString(configFileData, "acrConfigs", "acrClientID") acrPassword = getNestedJSONString(configFileData, "acrConfigs", "acrPassword")