diff --git a/contrib/pkg/utils/aws/aws.go b/contrib/pkg/utils/aws/aws.go index 2a516d4b89f..80b0b746879 100644 --- a/contrib/pkg/utils/aws/aws.go +++ b/contrib/pkg/utils/aws/aws.go @@ -1,10 +1,12 @@ package aws import ( + "errors" "os" "path/filepath" "github.com/openshift/hive/contrib/pkg/utils" + "github.com/openshift/hive/pkg/awsclient" "github.com/openshift/hive/pkg/constants" log "github.com/sirupsen/logrus" ini "gopkg.in/ini.v1" @@ -45,6 +47,16 @@ func GetAWSCreds(credsFile, defaultCredsFile string) (string, string, error) { return accessKeyIDValue.String(), secretAccessKeyValue.String(), nil } +var awsConfigForbidCredentialProcess utils.ProjectToDirFileFilter = func(key string, contents []byte) (basename string, newContents []byte, err error) { + // First, only process aws_config + bn, newContents, err := utils.ProjectOnlyTheseKeys(constants.AWSConfigSecretKey)(key, contents) + // If that passed, scrub for credential_process + if err == nil && bn != "" && awsclient.ContainsCredentialProcess(newContents) { + return "", nil, errors.New("credential_process is insecure and thus forbidden") + } + return bn, newContents, err +} + // ConfigureCreds loads a secret designated by the environment variables CLUSTERDEPLOYMENT_NAMESPACE // and CREDS_SECRET_NAME and configures AWS credential environment variables and config files // accordingly. @@ -61,11 +73,11 @@ func ConfigureCreds(c client.Client) { os.Setenv("AWS_SECRET_ACCESS_KEY", secret) } if config := credsSecret.Data[constants.AWSConfigSecretKey]; len(config) != 0 { - // Lay this down as a file - utils.ProjectToDir(credsSecret, constants.AWSCredsMount, constants.AWSConfigSecretKey) + // Lay this down as a file, but forbid credential_process + utils.ProjectToDir(credsSecret, constants.AWSCredsMount, awsConfigForbidCredentialProcess) os.Setenv("AWS_CONFIG_FILE", filepath.Join(constants.AWSCredsMount, constants.AWSConfigSecretKey)) } - // Allow credential_process in the config file + // This would normally allow credential_process in the config file, but we checked for that above. os.Setenv("AWS_SDK_LOAD_CONFIG", "true") // Install cluster proxy trusted CA bundle utils.InstallCerts(constants.TrustedCABundleDir) diff --git a/contrib/pkg/utils/azure/azure.go b/contrib/pkg/utils/azure/azure.go index bf252533d97..c26ac7bc150 100644 --- a/contrib/pkg/utils/azure/azure.go +++ b/contrib/pkg/utils/azure/azure.go @@ -36,7 +36,7 @@ func ConfigureCreds(c client.Client) { if credsSecret == nil { return } - utils.ProjectToDir(credsSecret, constants.AzureCredentialsDir) + utils.ProjectToDir(credsSecret, constants.AzureCredentialsDir, nil) os.Setenv(constants.AzureCredentialsEnvVar, constants.AzureCredentialsDir+"/"+constants.AzureCredentialsName) // Install cluster proxy trusted CA bundle utils.InstallCerts(constants.TrustedCABundleDir) diff --git a/contrib/pkg/utils/gcp/gcp.go b/contrib/pkg/utils/gcp/gcp.go index 94693e1f980..b3be4616ad3 100644 --- a/contrib/pkg/utils/gcp/gcp.go +++ b/contrib/pkg/utils/gcp/gcp.go @@ -37,7 +37,7 @@ func ConfigureCreds(c client.Client) { if credsSecret == nil { return } - utils.ProjectToDir(credsSecret, constants.GCPCredentialsDir) + utils.ProjectToDir(credsSecret, constants.GCPCredentialsDir, nil) os.Setenv("GOOGLE_CREDENTIALS", constants.GCPCredentialsDir+"/"+constants.GCPCredentialsName) // Install cluster proxy trusted CA bundle utils.InstallCerts(constants.TrustedCABundleDir) diff --git a/contrib/pkg/utils/generic.go b/contrib/pkg/utils/generic.go index 31240ad2241..09403f235a8 100644 --- a/contrib/pkg/utils/generic.go +++ b/contrib/pkg/utils/generic.go @@ -150,16 +150,56 @@ func loadOrDie(c client.Client, nameEnvKey string, obj client.Object) bool { } +// ProjectToDirFileFilter is run by ProjectToDir for each key found in the obj. +// If the second return is an error, ProjectToDir will panic with it. Otherwise: +// If ProjectToDir should create the file, the first return should be the base +// name of the file in which the newContents should be written. +// If the file should be skipped, the first return should be the empty string. +type ProjectToDirFileFilter func(key string, contents []byte) (basename string, newContents []byte, err error) + +// projectDefault is a ProjectToDirFileFilter that causes ProjectToDir to create +// files for all keys in the obj, naming each file the same as its key. +var projectDefault ProjectToDirFileFilter = func(key string, contents []byte) (string, []byte, error) { + return key, contents, nil +} + +// ProjectOnlyTheseKeys returns a ProjectToDirFileFilter that instructs ProjectToDir +// to create only the files with the specified keys. Each file's name will be the +// same as the key. The error return is always nil. +func ProjectOnlyTheseKeys(keys ...string) ProjectToDirFileFilter { + return func(key string, contents []byte) (string, []byte, error) { + if len(keys) == 0 { + // Caller should use nil (projectDefault) instead, but meh. + return key, contents, nil + } + if slice.ContainsString(keys, key, nil) { + // A match, project the file with the key as the basename and unchanged contents + return key, contents, nil + } + // No match; skip this file + return "", nil, nil + } +} + // ProjectToDir simulates what happens when you mount a secret or configmap as a volume on a pod, creating // files named after each key under `dir` and populating them with the contents represented by the values. -func ProjectToDir(obj client.Object, dir string, keys ...string) { - write := func(filename string, bytes []byte) { - if len(keys) != 0 && !slice.ContainsString(keys, filename, nil) { +// This default behavior can be modified by specifying a non-nil filter to validate, skip, and/or rename +// the file corresponding to each key. +func ProjectToDir(obj client.Object, dir string, filter ProjectToDirFileFilter) { + write := func(key string, bytes []byte) { + if filter == nil { + filter = projectDefault + } + filename, newBytes, err := filter(key, bytes) + if err != nil { + panic(err) + } + if filename == "" { // Skip this key return } path := filepath.Join(dir, filename) - if err := os.WriteFile(path, bytes, 0400); err != nil { + if err := os.WriteFile(path, newBytes, 0400); err != nil { log.WithError(err).WithField("path", path).Fatal("Failed to write file") } } diff --git a/contrib/pkg/utils/openstack/openstack.go b/contrib/pkg/utils/openstack/openstack.go index 48c33bde3df..26e2d313700 100644 --- a/contrib/pkg/utils/openstack/openstack.go +++ b/contrib/pkg/utils/openstack/openstack.go @@ -39,10 +39,10 @@ func GetCreds(credsFile string) ([]byte, error) { // CREDS_SECRET_NAME, and CERTS_SECRET_NAME and configures OpenStack credential config files accordingly. func ConfigureCreds(c client.Client) { if credsSecret := utils.LoadSecretOrDie(c, "CREDS_SECRET_NAME"); credsSecret != nil { - utils.ProjectToDir(credsSecret, constants.OpenStackCredentialsDir) + utils.ProjectToDir(credsSecret, constants.OpenStackCredentialsDir, nil) } if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil { - utils.ProjectToDir(certsSecret, constants.OpenStackCertificatesDir) + utils.ProjectToDir(certsSecret, constants.OpenStackCertificatesDir, nil) utils.InstallCerts(constants.OpenStackCertificatesDir) } // Install cluster proxy trusted CA bundle diff --git a/contrib/pkg/utils/ovirt/ovirt.go b/contrib/pkg/utils/ovirt/ovirt.go index cc44a3feb59..e859220423a 100644 --- a/contrib/pkg/utils/ovirt/ovirt.go +++ b/contrib/pkg/utils/ovirt/ovirt.go @@ -36,11 +36,11 @@ func GetCreds(credsFile string) ([]byte, error) { // and config files accordingly. func ConfigureCreds(c client.Client) { if credsSecret := utils.LoadSecretOrDie(c, "CREDS_SECRET_NAME"); credsSecret != nil { - utils.ProjectToDir(credsSecret, constants.OvirtCredentialsDir) + utils.ProjectToDir(credsSecret, constants.OvirtCredentialsDir, nil) os.Setenv(constants.OvirtConfigEnvVar, constants.OvirtCredentialsDir+"/"+constants.OvirtCredentialsName) } if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil { - utils.ProjectToDir(certsSecret, constants.OvirtCertificatesDir) + utils.ProjectToDir(certsSecret, constants.OvirtCertificatesDir, nil) utils.InstallCerts(constants.OvirtCertificatesDir) } // Install cluster proxy trusted CA bundle diff --git a/contrib/pkg/utils/vsphere/vsphere.go b/contrib/pkg/utils/vsphere/vsphere.go index 2bc77a62691..accf3290442 100644 --- a/contrib/pkg/utils/vsphere/vsphere.go +++ b/contrib/pkg/utils/vsphere/vsphere.go @@ -21,10 +21,10 @@ func ConfigureCreds(c client.Client) { } // NOTE: I think this is only used for terminateWhenFilesChange(), which we don't really // care about anymore. Can we get rid of it? - utils.ProjectToDir(credsSecret, constants.VSphereCredentialsDir) + utils.ProjectToDir(credsSecret, constants.VSphereCredentialsDir, nil) } if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil { - utils.ProjectToDir(certsSecret, constants.VSphereCertificatesDir) + utils.ProjectToDir(certsSecret, constants.VSphereCertificatesDir, nil) utils.InstallCerts(constants.VSphereCertificatesDir) } // Install cluster proxy trusted CA bundle diff --git a/docs/awsassumerolecreds.md b/docs/awsassumerolecreds.md index f25330be715..ea1195e0013 100644 --- a/docs/awsassumerolecreds.md +++ b/docs/awsassumerolecreds.md @@ -26,7 +26,7 @@ Let's create a Secret in `hive` (target namespace in HiveConfig) with credential ```console $ cat aws-service-provider-config -[default] +[profile source] aws_access_key_id = XXXXXX aws_secret_access_key = XXXXX role_arn = arn:aws:iam::123456:role/hive-aws-service-provider diff --git a/pkg/awsclient/client.go b/pkg/awsclient/client.go index f27469b5597..997ee2c52c3 100644 --- a/pkg/awsclient/client.go +++ b/pkg/awsclient/client.go @@ -5,6 +5,8 @@ import ( "context" "fmt" "os" + "regexp" + "strings" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -513,7 +515,10 @@ func NewSessionFromSecret(secret *corev1.Secret, region string) (*session.Sessio // Special case to not use a secret to gather credentials. if secret != nil { - config := awsCLIConfigFromSecret(secret) + config, err := awsCLIConfigFromSecret(secret) + if err != nil { + return nil, err + } f, err := os.CreateTemp("", "hive-aws-config") if err != nil { return nil, err @@ -542,18 +547,30 @@ func NewSessionFromSecret(secret *corev1.Secret, region string) (*session.Sessio return s, nil } +var credentialProcessRE = regexp.MustCompile(`(?i)\bcredential_process\b`) + +func ContainsCredentialProcess(config []byte) bool { + return len(credentialProcessRE.Find(config)) != 0 +} + // awsCLIConfigFromSecret returns an AWS CLI config using the data available in the secret. -func awsCLIConfigFromSecret(secret *corev1.Secret) []byte { +func awsCLIConfigFromSecret(secret *corev1.Secret) ([]byte, error) { if config, ok := secret.Data[constants.AWSConfigSecretKey]; ok { - return config + if ContainsCredentialProcess(config) { + return nil, errors.New("credential_process is insecure and thus forbidden") + } + return config, nil } buf := &bytes.Buffer{} fmt.Fprint(buf, "[default]\n") for k, v := range secret.Data { + if strings.ToLower(k) == "credential_process" { + return nil, errors.New("credential_process is insecure and thus forbidden") + } fmt.Fprintf(buf, "%s = %s\n", k, v) } - return buf.Bytes() + return buf.Bytes(), nil } func awsChinaEndpointResolver(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { diff --git a/pkg/controller/argocdregister/argocdregister_controller.go b/pkg/controller/argocdregister/argocdregister_controller.go index 6a931956be4..6d4b6f66b92 100644 --- a/pkg/controller/argocdregister/argocdregister_controller.go +++ b/pkg/controller/argocdregister/argocdregister_controller.go @@ -211,7 +211,7 @@ func (r *ArgoCDRegisterController) reconcileCluster(cd *hivev1.ClusterDeployment return reconcile.Result{}, nil } - kubeConfigSecretName := cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name + kubeConfigSecretName := cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name // HIVE-2485 ✓ argoCDServerConfigBytes, err := r.generateArgoCDServerConfig(kubeConfigSecretName, cd.Namespace, argoCDNamespace, cdLog) if err != nil { return reconcile.Result{}, err @@ -320,7 +320,7 @@ func (r *ArgoCDRegisterController) loadArgoCDServiceAccountToken(argoCDNamespace } func (r *ArgoCDRegisterController) generateArgoCDServerConfig(kubeconfigSecretName, kubeConfigSecretNamespace, argoCDNamespace string, cdLog log.FieldLogger) ([]byte, error) { - kubeconfig, err := r.loadSecretData(kubeconfigSecretName, kubeConfigSecretNamespace, adminKubeConfigKey) + kubeconfig, err := controllerutils.LoadSecretData(r.Client, kubeconfigSecretName, kubeConfigSecretNamespace, adminKubeConfigKey) if err != nil { cdLog.WithError(err).Error("unable to load cluster admin kubeconfig") return nil, err @@ -375,19 +375,6 @@ func tlsClientConfigBuilderFunc(kubeConfig clientcmd.ClientConfig, cdLog log.Fie return tlsClientConfig, nil } -func (r *ArgoCDRegisterController) loadSecretData(secretName, namespace, dataKey string) (string, error) { - s := &corev1.Secret{} - err := r.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, s) - if err != nil { - return "", err - } - retStr, ok := s.Data[dataKey] - if !ok { - return "", fmt.Errorf("secret %s did not contain key %s", secretName, dataKey) - } - return string(retStr), nil -} - // getPredictableSecretName generates a unique secret name by hashing the server API URL, // which is required as all cluster secrets land in the argocd namespace. // This code matches what is presently done in ArgoCD (util/db/cluster.go). However the actual diff --git a/pkg/controller/awsprivatelink/awsprivatelink_controller.go b/pkg/controller/awsprivatelink/awsprivatelink_controller.go index 82f23f7e32c..e112daf12ea 100644 --- a/pkg/controller/awsprivatelink/awsprivatelink_controller.go +++ b/pkg/controller/awsprivatelink/awsprivatelink_controller.go @@ -25,8 +25,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/flowcontrol" "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" @@ -307,7 +305,13 @@ func (r *ReconcileAWSPrivateLink) Reconcile(ctx context.Context, request reconci return reconcile.Result{}, nil } - return r.reconcilePrivateLink(cd, &hivev1.ClusterMetadata{InfraID: *cp.Spec.InfraID, AdminKubeconfigSecretRef: *cp.Spec.AdminKubeconfigSecretRef}, logger) + return r.reconcilePrivateLink( + cd, + &hivev1.ClusterMetadata{ + InfraID: *cp.Spec.InfraID, + AdminKubeconfigSecretRef: *cp.Spec.AdminKubeconfigSecretRef, // HIVE-2585: via ClusterMetadata + }, + logger) } // shouldSync returns if we should sync the desired ClusterDeployment. If it returns false, it also returns @@ -539,7 +543,7 @@ func (r *ReconcileAWSPrivateLink) reconcilePrivateLink(cd *hivev1.ClusterDeploym // Figure out the API address for cluster. apiDomain, err := initialURL(r.Client, - client.ObjectKey{Namespace: cd.Namespace, Name: clusterMetadata.AdminKubeconfigSecretRef.Name}) + client.ObjectKey{Namespace: cd.Namespace, Name: clusterMetadata.AdminKubeconfigSecretRef.Name}) // HIVE-2485 ✓ if err != nil { logger.WithError(err).Error("could not get API URL from kubeconfig") @@ -1274,7 +1278,7 @@ func newAWSClient(r *ReconcileAWSPrivateLink, cd *hivev1.ClusterDeployment) (*aw }, AssumeRole: &awsclient.AssumeRoleCredentialsSource{ SecretRef: corev1.SecretReference{ - Name: os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar), + Name: controllerutils.AWSServiceProviderSecretName(""), Namespace: controllerutils.GetHiveNamespace(), }, Role: cd.Spec.Platform.AWS.CredentialsAssumeRole, @@ -1309,7 +1313,7 @@ func initialURL(c client.Client, key client.ObjectKey) (string, error) { ); err != nil { return "", err } - cfg, err := restConfigFromSecret(kubeconfigSecret) + cfg, err := controllerutils.RestConfigFromSecret(kubeconfigSecret, true) if err != nil { return "", errors.Wrap(err, "failed to load the kubeconfig") } @@ -1321,22 +1325,6 @@ func initialURL(c client.Client, key client.ObjectKey) (string, error) { return strings.TrimSuffix(u.Hostname(), "."), nil } -func restConfigFromSecret(kubeconfigSecret *corev1.Secret) (*rest.Config, error) { - kubeconfigData := kubeconfigSecret.Data[constants.RawKubeconfigSecretKey] - if len(kubeconfigData) == 0 { - kubeconfigData = kubeconfigSecret.Data[constants.KubeconfigSecretKey] - } - if len(kubeconfigData) == 0 { - return nil, errors.New("kubeconfig secret does not contain necessary data") - } - config, err := clientcmd.Load(kubeconfigData) - if err != nil { - return nil, err - } - kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{}) - return kubeConfig.ClientConfig() -} - // awsErrCodeEquals returns true if the error matches all these conditions: // - err is of type awserr.Error // - Error.Code() equals code diff --git a/pkg/controller/awsprivatelink/cleanup.go b/pkg/controller/awsprivatelink/cleanup.go index caa35d21bb5..d458a634272 100644 --- a/pkg/controller/awsprivatelink/cleanup.go +++ b/pkg/controller/awsprivatelink/cleanup.go @@ -61,7 +61,7 @@ func (r *ReconcileAWSPrivateLink) cleanupPreviousProvisionAttempt(cd *hivev1.Clu } metadata := &hivev1.ClusterMetadata{ InfraID: *cp.Spec.PrevInfraID, - AdminKubeconfigSecretRef: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef, + AdminKubeconfigSecretRef: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef, // HIVE-2485: via ClusterMetadata } if err := r.cleanupPrivateLink(cd, metadata, logger); err != nil { @@ -138,7 +138,7 @@ func (r *ReconcileAWSPrivateLink) cleanupHostedZone(awsClient awsclient.Client, if hzID == "" { // since we don't have the hz ID, we try to discover it to prevent leaks apiDomain, err := initialURL(r.Client, - client.ObjectKey{Namespace: cd.Namespace, Name: metadata.AdminKubeconfigSecretRef.Name}) + client.ObjectKey{Namespace: cd.Namespace, Name: metadata.AdminKubeconfigSecretRef.Name}) // HIVE-2485 ✓ if apierrors.IsNotFound(err) { logger.Info("no hostedZoneID in status and admin kubeconfig does not exist, skipping hosted zone cleanup") return nil diff --git a/pkg/controller/clusterclaim/clusterclaim_controller.go b/pkg/controller/clusterclaim/clusterclaim_controller.go index b246cb7c958..7fc9e0c0f38 100644 --- a/pkg/controller/clusterclaim/clusterclaim_controller.go +++ b/pkg/controller/clusterclaim/clusterclaim_controller.go @@ -548,7 +548,7 @@ func (r *ReconcileClusterClaim) applyHiveClaimOwnerRole(claim *hivev1.ClusterCla APIGroups: []string{corev1.GroupName}, Resources: []string{"secrets"}, ResourceNames: []string{ - cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name, + cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name, // HIVE-2485: n/a cd.Spec.ClusterMetadata.AdminPasswordSecretRef.Name, }, Verbs: []string{"get"}, diff --git a/pkg/controller/clusterdeployment/clusterdeployment_controller.go b/pkg/controller/clusterdeployment/clusterdeployment_controller.go index f1fc6113a42..6561731fdfb 100644 --- a/pkg/controller/clusterdeployment/clusterdeployment_controller.go +++ b/pkg/controller/clusterdeployment/clusterdeployment_controller.go @@ -439,7 +439,14 @@ func (r *ReconcileClusterDeployment) addAdditionalKubeconfigCAs(cd *hivev1.Clust cdLog log.FieldLogger) error { adminKubeconfigSecret := &corev1.Secret{} - if err := r.Get(context.Background(), types.NamespacedName{Namespace: cd.Namespace, Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name}, adminKubeconfigSecret); err != nil { + if err := r.Get( + context.Background(), + types.NamespacedName{ + Namespace: cd.Namespace, + // HIVE-2485: No need to scrub here + Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name, + }, + adminKubeconfigSecret); err != nil { cdLog.WithError(err).Error("failed to get admin kubeconfig secret") return err } @@ -642,6 +649,7 @@ func (r *ReconcileClusterDeployment) reconcile(request reconcile.Request, cd *hi } // Add cluster deployment as additional owner reference to admin secrets + // HIVE-2485: No need to scrub here if err := r.addOwnershipToSecret(cd, cdLog, cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name); err != nil { return reconcile.Result{}, err } diff --git a/pkg/controller/clusterdeployment/clusterinstalls.go b/pkg/controller/clusterdeployment/clusterinstalls.go index 773b0e8c1c5..b09c58a5ecb 100644 --- a/pkg/controller/clusterdeployment/clusterinstalls.go +++ b/pkg/controller/clusterdeployment/clusterinstalls.go @@ -54,7 +54,7 @@ func (r *ReconcileClusterDeployment) reconcileExistingInstallingClusterInstall(c if met := ci.Spec.ClusterMetadata; met != nil && met.InfraID != "" && met.ClusterID != "" && - met.AdminKubeconfigSecretRef.Name != "" && + met.AdminKubeconfigSecretRef.Name != "" && // HIVE-2485: via ClusterMetadata met.AdminPasswordSecretRef != nil && met.AdminPasswordSecretRef.Name != "" { if !reflect.DeepEqual(cd.Spec.ClusterMetadata, ci.Spec.ClusterMetadata) { diff --git a/pkg/controller/clusterdeployment/clusterprovisions.go b/pkg/controller/clusterdeployment/clusterprovisions.go index 6ac1b361ee6..fdd1855131b 100644 --- a/pkg/controller/clusterdeployment/clusterprovisions.go +++ b/pkg/controller/clusterdeployment/clusterprovisions.go @@ -209,12 +209,8 @@ func (r *ReconcileClusterDeployment) startNewProvision( } if err := r.setupAWSCredentialForAssumeRole(cd); err != nil { - if !apierrors.IsAlreadyExists(err) { - // Couldn't create the assume role credential secret for a reason other than it already exists. - // If the secret already exists, then we should just use that secret. - logger.WithError(err).Error("could not create AWS assume role credential secret") - return reconcile.Result{}, err - } + logger.WithError(err).Error("could not create or update AWS assume role credential secret") + return reconcile.Result{}, err } r.expectations.ExpectCreations(types.NamespacedName{Namespace: cd.Namespace, Name: cd.Name}.String(), 1) @@ -302,7 +298,7 @@ func (r *ReconcileClusterDeployment) reconcileExistingProvision(cd *hivev1.Clust clusterMetadata.ClusterID = *provision.Spec.ClusterID } if provision.Spec.AdminKubeconfigSecretRef != nil { - clusterMetadata.AdminKubeconfigSecretRef = *provision.Spec.AdminKubeconfigSecretRef + clusterMetadata.AdminKubeconfigSecretRef = *provision.Spec.AdminKubeconfigSecretRef // HIVE-2485: via ClusterMetadata } if provision.Spec.AdminPasswordSecretRef != nil { clusterMetadata.AdminPasswordSecretRef = provision.Spec.AdminPasswordSecretRef @@ -626,7 +622,7 @@ func getInstallLogEnvVars(secretPrefix string) ([]corev1.EnvVar, error) { func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeployment, secretPrefix string) []corev1.EnvVar { var extraEnvVars []corev1.EnvVar - spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + spSecretName := controllerutils.AWSServiceProviderSecretName(secretPrefix) if spSecretName == "" { return extraEnvVars } @@ -637,7 +633,7 @@ func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeployment, secretPrefix str extraEnvVars = append(extraEnvVars, corev1.EnvVar{ Name: constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar, - Value: secretPrefix + "-" + spSecretName, + Value: spSecretName, }) return extraEnvVars } @@ -650,7 +646,7 @@ func (r *ReconcileClusterDeployment) setupAWSCredentialForAssumeRole(cd *hivev1. return nil } - return install.AWSAssumeRoleCLIConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) + return install.AWSAssumeRoleConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) } func (r *ReconcileClusterDeployment) watchClusterProvisions(c controller.Controller) error { diff --git a/pkg/controller/clusterdeprovision/awsactuator.go b/pkg/controller/clusterdeprovision/awsactuator.go index b6c12229316..7786664a95a 100644 --- a/pkg/controller/clusterdeprovision/awsactuator.go +++ b/pkg/controller/clusterdeprovision/awsactuator.go @@ -1,8 +1,6 @@ package clusterdeprovision import ( - "os" - log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -11,7 +9,6 @@ import ( hivev1 "github.com/openshift/hive/apis/hive/v1" awsclient "github.com/openshift/hive/pkg/awsclient" - "github.com/openshift/hive/pkg/constants" controllerutils "github.com/openshift/hive/pkg/controller/utils" ) @@ -61,7 +58,7 @@ func getAWSClient(cd *hivev1.ClusterDeprovision, c client.Client, logger log.Fie }, AssumeRole: &awsclient.AssumeRoleCredentialsSource{ SecretRef: corev1.SecretReference{ - Name: os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar), + Name: controllerutils.AWSServiceProviderSecretName(""), Namespace: controllerutils.GetHiveNamespace(), }, Role: cd.Spec.Platform.AWS.CredentialsAssumeRole, diff --git a/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go b/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go index 68687ec4773..089bb051519 100644 --- a/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go +++ b/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go @@ -274,12 +274,8 @@ func (r *ReconcileClusterDeprovision) Reconcile(ctx context.Context, request rec } if err := r.setupAWSCredentialForAssumeRole(instance); err != nil { - if !errors.IsAlreadyExists(err) { - // Couldn't create the assume role credentials secret for a reason other than it already exists. - // If the secret already exists, then we should just use that secret. - rLog.WithError(err).Error("could not create assume role AWS secret") - return reconcile.Result{}, err - } + rLog.WithError(err).Error("could not create or update assume role AWS secret") + return reconcile.Result{}, err } if err := controllerutils.SetupClusterUninstallServiceAccount(r, cd.Namespace, rLog); err != nil { @@ -435,7 +431,7 @@ func (r *ReconcileClusterDeprovision) getActuator(cd *hivev1.ClusterDeprovision) func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeprovision, secretPrefix string) []corev1.EnvVar { var extraEnvVars []corev1.EnvVar - spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + spSecretName := controllerutils.AWSServiceProviderSecretName(secretPrefix) if spSecretName == "" { return extraEnvVars } @@ -446,7 +442,7 @@ func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeprovision, secretPrefix st extraEnvVars = append(extraEnvVars, corev1.EnvVar{ Name: constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar, - Value: secretPrefix + "-" + spSecretName, + Value: spSecretName, }) return extraEnvVars } @@ -459,6 +455,6 @@ func (r *ReconcileClusterDeprovision) setupAWSCredentialForAssumeRole(cd *hivev1 return nil } - return install.AWSAssumeRoleCLIConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) + return install.AWSAssumeRoleConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) } diff --git a/pkg/controller/dnszone/dnszone_controller.go b/pkg/controller/dnszone/dnszone_controller.go index 30ef654b1e0..a50082fee36 100644 --- a/pkg/controller/dnszone/dnszone_controller.go +++ b/pkg/controller/dnszone/dnszone_controller.go @@ -16,7 +16,6 @@ import ( hivev1 "github.com/openshift/hive/apis/hive/v1" awsclient "github.com/openshift/hive/pkg/awsclient" "github.com/openshift/hive/pkg/azureclient" - "github.com/openshift/hive/pkg/constants" hivemetrics "github.com/openshift/hive/pkg/controller/metrics" controllerutils "github.com/openshift/hive/pkg/controller/utils" gcpclient "github.com/openshift/hive/pkg/gcpclient" @@ -402,7 +401,7 @@ func (r *ReconcileDNSZone) getActuator(dnsZone *hivev1.DNSZone, dnsLog log.Field AssumeRole: &awsclient.AssumeRoleCredentialsSource{ SecretRef: corev1.SecretReference{ Namespace: controllerutils.GetHiveNamespace(), - Name: os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar), + Name: controllerutils.AWSServiceProviderSecretName(""), }, Role: dnsZone.Spec.AWS.CredentialsAssumeRole, }, diff --git a/pkg/controller/hibernation/aws_actuator.go b/pkg/controller/hibernation/aws_actuator.go index 67a64891f1b..2455a6d3974 100644 --- a/pkg/controller/hibernation/aws_actuator.go +++ b/pkg/controller/hibernation/aws_actuator.go @@ -3,7 +3,6 @@ package hibernation import ( "context" "fmt" - "os" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -18,7 +17,6 @@ import ( hivev1 "github.com/openshift/hive/apis/hive/v1" awsclient "github.com/openshift/hive/pkg/awsclient" - "github.com/openshift/hive/pkg/constants" controllerutils "github.com/openshift/hive/pkg/controller/utils" ) @@ -261,7 +259,7 @@ func getAWSClient(cd *hivev1.ClusterDeployment, c client.Client, logger log.Fiel }, AssumeRole: &awsclient.AssumeRoleCredentialsSource{ SecretRef: corev1.SecretReference{ - Name: os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar), + Name: controllerutils.AWSServiceProviderSecretName(""), Namespace: controllerutils.GetHiveNamespace(), }, Role: cd.Spec.Platform.AWS.CredentialsAssumeRole, diff --git a/pkg/controller/machinepool/machinepool_controller.go b/pkg/controller/machinepool/machinepool_controller.go index 69d207cd756..a1d309ecf93 100644 --- a/pkg/controller/machinepool/machinepool_controller.go +++ b/pkg/controller/machinepool/machinepool_controller.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "os" "reflect" "strings" "time" @@ -1111,7 +1110,7 @@ func (r *ReconcileMachinePool) createActuator( AssumeRole: &awsclient.AssumeRoleCredentialsSource{ SecretRef: corev1.SecretReference{ Namespace: controllerutils.GetHiveNamespace(), - Name: os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar), + Name: controllerutils.AWSServiceProviderSecretName(""), }, Role: cd.Spec.Platform.AWS.CredentialsAssumeRole, }, diff --git a/pkg/controller/utils/secrets.go b/pkg/controller/utils/secrets.go index 45bd125eda8..b17138fe725 100644 --- a/pkg/controller/utils/secrets.go +++ b/pkg/controller/utils/secrets.go @@ -2,23 +2,94 @@ package utils import ( "context" + "errors" "fmt" + "os" + "github.com/openshift/hive/pkg/constants" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" "sigs.k8s.io/controller-runtime/pkg/client" ) -// LoadSecretData loads a given secret key and returns it's data as a string. +// LoadSecretData loads a given secret key and returns its data as a string. func LoadSecretData(c client.Client, secretName, namespace, dataKey string) (string, error) { s := &corev1.Secret{} err := c.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, s) if err != nil { return "", err } - retStr, ok := s.Data[dataKey] + retBytes, ok := s.Data[dataKey] if !ok { return "", fmt.Errorf("secret %s did not contain key %s", secretName, dataKey) } - return string(retStr), nil + switch dataKey { + case constants.KubeconfigSecretKey, constants.RawKubeconfigSecretKey: + if _, err := validateKubeconfig(retBytes); err != nil { + return "", err + } + } + return string(retBytes), nil +} + +// RestConfigFromSecret accepts a Secret containing `kubeconfig` and/or `raw-kubeconfig` keys +// and returns a rest.Config loaded therefrom. +// If tryRaw is true, we will look for `raw-kubeconfig` first and use it if present, falling +// back to `kubeconfig` otherwise. +// The error return is non-nil if: +// - The Secret's Data does not contain the [raw-]kubeconfig key(s) +// - The kubeconfig data cannot be Load()ed +// - The kubeconfig is insecure +func RestConfigFromSecret(kubeconfigSecret *corev1.Secret, tryRaw bool) (*rest.Config, error) { + var kubeconfigData []byte + if tryRaw { + kubeconfigData = kubeconfigSecret.Data[constants.RawKubeconfigSecretKey] + } + if len(kubeconfigData) == 0 { + kubeconfigData = kubeconfigSecret.Data[constants.KubeconfigSecretKey] + } + if len(kubeconfigData) == 0 { + return nil, errors.New("kubeconfig secret does not contain necessary data") + } + config, err := validateKubeconfig(kubeconfigData) + if err != nil { + return nil, err + } + kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{}) + return kubeConfig.ClientConfig() +} + +// validateKubeconfig ensures the kubeconfig represented by kc does not use insecure paths. +// HIVE-2485 +func validateKubeconfig(kc []byte) (*api.Config, error) { + config, err := clientcmd.Load(kc) + if err != nil { + return nil, err + } + for k, ai := range config.AuthInfos { + if ai.Exec != nil { + return nil, fmt.Errorf("insecure exec in AuthInfos[%s]", k) + } + } + return config, nil +} + +// Generate the name of the Secret containing AWS Service Provider configuration. The +// prefix should be specified when the env var is known to refer to the global (hive +// namespace) Secret to convert it to the local (CD-specific) name. +// Beware of recursion: we're overloading the env var to refer to both the secret in the +// hive namespace and that in the CD-specific namespace. +func AWSServiceProviderSecretName(prefix string) string { + spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + // If no secret is given via env, this is n/a + if spSecretName == "" { + return "" + } + if prefix == "" { + return spSecretName + } + return prefix + "-" + spSecretName } diff --git a/pkg/install/generate.go b/pkg/install/generate.go index 90de4215c8b..4a621a67961 100644 --- a/pkg/install/generate.go +++ b/pkg/install/generate.go @@ -3,7 +3,7 @@ package install import ( "context" "fmt" - "os" + "reflect" "strings" "time" @@ -14,6 +14,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -49,7 +50,7 @@ func AWSAssumeRoleSecretName(secretPrefix string) string { func CopyAWSServiceProviderSecret(client client.Client, destNamespace string, envVars []corev1.EnvVar, owner metav1.Object, scheme *runtime.Scheme) error { hiveNS := controllerutils.GetHiveNamespace() - spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + spSecretName := controllerutils.AWSServiceProviderSecretName("") if spSecretName == "" { // If the src secret reference wasn't found, then don't attempt to copy the secret. return nil @@ -73,42 +74,79 @@ func CopyAWSServiceProviderSecret(client client.Client, destNamespace string, en return controllerutils.CopySecret(client, src, dest, owner, scheme) } -// AWSAssumeRoleCLIConfig creates a secret that can assume the role using the hiveutil -// credential_process helper. -func AWSAssumeRoleCLIConfig(client client.Client, role *hivev1aws.AssumeRole, secretName, secretNamespace string, owner metav1.Object, scheme *runtime.Scheme) error { - cmd := "/usr/bin/hiveutil" - args := []string{"install-manager", "aws-credentials"} - args = append(args, []string{"--namespace", secretNamespace}...) - args = append(args, []string{"--role-arn", role.RoleARN}...) - if role.ExternalID != "" { - args = append(args, []string{"--external-id", role.ExternalID}...) +// AWSAssumeRoleConfig creates or updates a secret with an AWS credentials file containing: +// - Role configuration for AssumeRole, pointing to... +// - A profile containing the source credentials for AssumeRole. +func AWSAssumeRoleConfig(client client.Client, role *hivev1aws.AssumeRole, secretName, secretNamespace string, owner metav1.Object, scheme *runtime.Scheme) error { + + // Credentials source + credsSecret := &corev1.Secret{} + credsSecretName := controllerutils.AWSServiceProviderSecretName(owner.GetName()) + if err := client.Get( + context.TODO(), + types.NamespacedName{ + Namespace: secretNamespace, + Name: credsSecretName, + }, + credsSecret); err != nil { + return errors.Wrapf(err, "failed to load credentials source secret %s", credsSecretName) } + // The old credential_process flow documented creating this with [default]. + // For backward compatibility, accept that, but convert to [profile source]. + sourceProfile := strings.Replace(string(credsSecret.Data[constants.AWSConfigSecretKey]), `[default]`, `[profile source]`, 1) - cmd = fmt.Sprintf("%s %s", cmd, strings.Join(args, " ")) - - template := `[default] -credential_process = %s -` - data := fmt.Sprintf(template, cmd) + extID := "" + if role.ExternalID != "" { + extID = fmt.Sprintf("external_id = %s\n", role.ExternalID) + } + + // Build the config file + configFile := fmt.Sprintf(`[default] +source_profile = source +role_arn = %s +%s +%s +`, + role.RoleARN, extID, sourceProfile) + + // Load the config secret if it already exists + configSecret := &corev1.Secret{} + if err := client.Get(context.TODO(), types.NamespacedName{Namespace: secretNamespace, Name: secretName}, configSecret); err != nil { + if !apierrors.IsNotFound(err) { + return errors.Wrapf(err, "failed to load config secret %s", secretName) + } + // Not found -- build it + configSecret = &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: secretNamespace, + Name: secretName, + }, + Data: map[string][]byte{ + constants.AWSConfigSecretKey: []byte(configFile), + }, + } + if err := controllerutil.SetOwnerReference(owner, configSecret, scheme); err != nil { + return err + } + return client.Create(context.TODO(), configSecret) + } - secret := &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: secretNamespace, - Name: secretName, - }, - Data: map[string][]byte{ - constants.AWSConfigSecretKey: []byte(data), - }, + // Secret exists -- do we need to update it? Compare data and owner references. + origSecret := configSecret.DeepCopy() + configSecret.Data[constants.AWSConfigSecretKey] = []byte(configFile) + // SetOwnerReference is a no-op if the owner is already registered + if err := controllerutil.SetOwnerReference(owner, configSecret, scheme); err != nil { + return err } - if err := controllerutil.SetOwnerReference(owner, secret, scheme); err != nil { + if reflect.DeepEqual(origSecret, configSecret) { return nil } - return client.Create(context.TODO(), secret) + return client.Update(context.TODO(), configSecret) } // InstallerPodSpec generates a spec for an installer pod. diff --git a/pkg/installmanager/aws_credentials.go b/pkg/installmanager/aws_credentials.go deleted file mode 100644 index 3b1f497bd71..00000000000 --- a/pkg/installmanager/aws_credentials.go +++ /dev/null @@ -1,157 +0,0 @@ -package installmanager - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "os" - "time" - - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/credentials/stscreds" - "github.com/pkg/errors" - "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - contributils "github.com/openshift/hive/contrib/pkg/utils" - "github.com/openshift/hive/pkg/awsclient" - "github.com/openshift/hive/pkg/constants" -) - -// AWSCredentials is a supported external process credential provider as detailed in -// https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html -type AWSCredentials struct { - output io.WriteCloser - kubeClient client.Client - - ServiceProviderSecretName string - ServiceProviderSecretNamespace string - - RoleARN string - ExternalID string -} - -// NewInstallManagerAWSCredentials is the entrypoint to load credentials for AWS SDK -// using the service provider credentials. It supports the external process credential -// provider as mentioned in https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html -func NewInstallManagerAWSCredentials() *cobra.Command { - options := &AWSCredentials{} - cmd := &cobra.Command{ - Use: "aws-credentials", - Short: "Loads AWS credentials using the service provider credentials", - Long: "This loads the AWS credentials using the service provider credentials and then returns them in format defined in https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html", - Run: func(cmd *cobra.Command, args []string) { - if err := options.Complete(args); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - if err := options.Validate(); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - var err error - options.kubeClient, err = contributils.GetClient() - if err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - if err := options.Run(); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - }, - } - flags := cmd.Flags() - flags.StringVar(&options.ServiceProviderSecretNamespace, "namespace", "", "The namespace where the service provider secret is stored") - cmd.MarkFlagRequired("namespace") - flags.StringVar(&options.RoleARN, "role-arn", "", "The IAM role that should be assumed") - cmd.MarkFlagRequired("role-arn") - flags.StringVar(&options.ExternalID, "external-id", "", "External identifier required to assume the role specified.") - - return cmd -} - -// Validate the options -func (options *AWSCredentials) Validate() error { return nil } - -// Complete the options using the args -func (options *AWSCredentials) Complete(args []string) error { - options.output = os.Stdout - options.ServiceProviderSecretName = os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) - return nil -} - -// Run runs the command using the options. -func (options *AWSCredentials) Run() error { - var secret *corev1.Secret - if options.ServiceProviderSecretName != "" { - secret = &corev1.Secret{} - if err := options.kubeClient.Get(context.TODO(), - client.ObjectKey{Namespace: options.ServiceProviderSecretNamespace, Name: options.ServiceProviderSecretName}, - secret); err != nil { - return errors.Wrap(err, "failed to get the service provider secret") - } - } - - sess, err := awsclient.NewSessionFromSecret(secret, "") - if err != nil { - return errors.Wrap(err, "failed to create AWS session") - } - - duration := stscreds.DefaultDuration - creds := stscreds.NewCredentials(sess, options.RoleARN, func(p *stscreds.AssumeRoleProvider) { - p.Duration = duration - if options.ExternalID != "" { - p.ExternalID = &options.ExternalID - } - }) - v, err := creds.Get() - if err != nil { - return errors.Wrap(err, "failed to Assume the require role") - } - - resp, err := newCredentialProcessResponse(v, time.Now().Add(-1*time.Minute).Add(duration)) - if err != nil { - return errors.Wrap(err, "failed to create response for credential process") - } - _, err = fmt.Fprint(options.output, resp) - return err -} - -func newCredentialProcessResponse(v credentials.Value, expiry time.Time) (string, error) { - resp := &credentialProcessResponse{ - Version: 1, - AccessKeyID: v.AccessKeyID, - SecretAccessKey: v.SecretAccessKey, - SessionToken: v.SessionToken, - Expiration: &expiry, - } - outRaw, err := json.Marshal(resp) - if err != nil { - return "", errors.Wrap(err, "failed to create the credential process response") - } - outRawCompact := &bytes.Buffer{} - if err := json.Compact(outRawCompact, outRaw); err != nil { - return "", errors.Wrap(err, "failed to compact the JSON response") - } - - return outRawCompact.String(), nil -} - -type credentialProcessResponse struct { - Version int - AccessKeyID string `json:"AccessKeyId"` - SecretAccessKey string - SessionToken string - Expiration *time.Time -} diff --git a/pkg/installmanager/aws_credentials_test.go b/pkg/installmanager/aws_credentials_test.go deleted file mode 100644 index 635da340b77..00000000000 --- a/pkg/installmanager/aws_credentials_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package installmanager - -import ( - "testing" - "time" - - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -func TestNewCredentialProcessResponse(t *testing.T) { - scheme := runtime.NewScheme() - corev1.AddToScheme(scheme) - - cases := []struct { - name string - creds credentials.Value - resp string - }{{ - name: "valid credentials", - creds: credentials.Value{ - AccessKeyID: "ASX..ID...", - SecretAccessKey: "ASX..SECRET...", - SessionToken: "ASX..TOKEN...", - }, - resp: `{"Version":1,"AccessKeyId":"ASX..ID...","SecretAccessKey":"ASX..SECRET...","SessionToken":"ASX..TOKEN...","Expiration":"0001-01-01T00:00:00Z"}`, - }} - for _, test := range cases { - t.Run(test.name, func(t *testing.T) { - got, err := newCredentialProcessResponse(test.creds, time.Time{}) - require.NoError(t, err) - assert.Equal(t, test.resp, got) - }) - } -} diff --git a/pkg/installmanager/installmanager.go b/pkg/installmanager/installmanager.go index bf32d1e5a76..cd71bfa27a8 100644 --- a/pkg/installmanager/installmanager.go +++ b/pkg/installmanager/installmanager.go @@ -205,7 +205,6 @@ SSH_PRIV_KEY_PATH: File system path of a file containing the SSH private key cor flags.StringVar(&im.WorkDir, "work-dir", "/output", "directory to use for all input and output") flags.StringVar(&im.LogsDir, "logs-dir", "/logs", "directory to use for all installer logs") - cmd.AddCommand(NewInstallManagerAWSCredentials()) return cmd } @@ -549,26 +548,28 @@ func loadSecrets(m *InstallManager, cd *hivev1.ClusterDeployment) { } // Load up the install config and pull secret. These env vars are required; else we'll panic. - contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "INSTALLCONFIG_SECRET_NAME"), "/installconfig") - contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "PULLSECRET_SECRET_NAME"), "/pullsecret") + contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "INSTALLCONFIG_SECRET_NAME"), "/installconfig", nil) + contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "PULLSECRET_SECRET_NAME"), "/pullsecret", nil) // Additional manifests? Could come in on a Secret or a ConfigMap if manSecret := contributils.LoadSecretOrDie(m.DynamicClient, "MANIFESTS_SECRET_NAME"); manSecret != nil { - contributils.ProjectToDir(manSecret, "/manifests") + contributils.ProjectToDir(manSecret, "/manifests", nil) } else if manCM := contributils.LoadConfigMapOrDie(m.DynamicClient, "MANIFESTS_CONFIGMAP_NAME"); manCM != nil { - contributils.ProjectToDir(manCM, "/manifests") + contributils.ProjectToDir(manCM, "/manifests", nil) } // Custom BoundServiceAccountSigningKey if bsask := contributils.LoadSecretOrDie(m.DynamicClient, "BOUND_TOKEN_SIGNING_KEY_SECRET_NAME"); bsask != nil { - contributils.ProjectToDir(bsask, constants.BoundServiceAccountSigningKeyDir, constants.BoundServiceAccountSigningKeyFile) + contributils.ProjectToDir( + bsask, constants.BoundServiceAccountSigningKeyDir, + contributils.ProjectOnlyTheseKeys(constants.BoundServiceAccountSigningKeyFile)) os.Setenv(constants.BoundServiceAccountSigningKeyEnvVar, constants.BoundServiceAccountSigningKeyDir+"/"+constants.BoundServiceAccountSigningKeyFile) } // SSH private key if sshkey := contributils.LoadSecretOrDie(m.DynamicClient, "SSH_PRIVATE_KEY_SECRET_PATH"); sshkey != nil { - contributils.ProjectToDir(sshkey, constants.SSHPrivateKeyDir) + contributils.ProjectToDir(sshkey, constants.SSHPrivateKeyDir, nil) // TODO: Collapse this in initSSHKey os.Setenv(constants.SSHPrivKeyPathEnvVar, constants.SSHPrivateKeyDir+"/"+constants.SSHPrivateKeySecretKey) @@ -576,7 +577,7 @@ func loadSecrets(m *InstallManager, cd *hivev1.ClusterDeployment) { // BareMetal Libvirt SSH private key if sshkey := contributils.LoadSecretOrDie(m.DynamicClient, "LIBVIRT_SSH_KEYS_SECRET_NAME"); sshkey != nil { - contributils.ProjectToDir(sshkey, constants.LibvirtSSHPrivateKeyDir) + contributils.ProjectToDir(sshkey, constants.LibvirtSSHPrivateKeyDir, nil) os.Setenv(constants.LibvirtSSHPrivKeyPathEnvVar, constants.LibvirtSSHPrivateKeyDir+"/"+constants.SSHPrivateKeySecretKey) } diff --git a/pkg/remoteclient/kubeconfig.go b/pkg/remoteclient/kubeconfig.go index 505ec665650..39f773d76c9 100644 --- a/pkg/remoteclient/kubeconfig.go +++ b/pkg/remoteclient/kubeconfig.go @@ -1,6 +1,7 @@ package remoteclient import ( + "github.com/openshift/hive/pkg/controller/utils" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/dynamic" @@ -75,5 +76,5 @@ func (b *kubeconfigBuilder) UseSecondaryAPIURL() Builder { } func (b *kubeconfigBuilder) RESTConfig() (*rest.Config, error) { - return restConfigFromSecret(b.secret) + return utils.RestConfigFromSecret(b.secret, false) } diff --git a/pkg/remoteclient/remoteclient.go b/pkg/remoteclient/remoteclient.go index 693034209fb..f9812ec56c5 100644 --- a/pkg/remoteclient/remoteclient.go +++ b/pkg/remoteclient/remoteclient.go @@ -15,7 +15,6 @@ import ( "k8s.io/client-go/dynamic" kubeclient "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/client" openshiftapiv1 "github.com/openshift/api/config/v1" @@ -25,7 +24,6 @@ import ( autoscalingv1beta1 "github.com/openshift/cluster-autoscaler-operator/pkg/apis/autoscaling/v1beta1" hivev1 "github.com/openshift/hive/apis/hive/v1" - "github.com/openshift/hive/pkg/constants" "github.com/openshift/hive/pkg/controller/utils" ) @@ -305,23 +303,11 @@ func unadulteratedRESTConfig(c client.Client, cd *hivev1.ClusterDeployment) (*re kubeconfigSecret := &corev1.Secret{} if err := c.Get( context.Background(), + // HIVE-2485 ✓ client.ObjectKey{Namespace: cd.Namespace, Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name}, kubeconfigSecret, ); err != nil { return nil, errors.Wrap(err, "could not get admin kubeconfig secret") } - return restConfigFromSecret(kubeconfigSecret) -} - -func restConfigFromSecret(kubeconfigSecret *corev1.Secret) (*rest.Config, error) { - kubeconfigData, ok := kubeconfigSecret.Data[constants.KubeconfigSecretKey] - if !ok { - return nil, errors.Errorf("kubeconfig secret does not contain %q data", constants.KubeconfigSecretKey) - } - config, err := clientcmd.Load(kubeconfigData) - if err != nil { - return nil, err - } - kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{}) - return kubeConfig.ClientConfig() + return utils.RestConfigFromSecret(kubeconfigSecret, false) }