diff --git a/pkg/controller/argocdregister/argocdregister_controller.go b/pkg/controller/argocdregister/argocdregister_controller.go index 927d36ef394..602c1d25016 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 04afd70b8b9..6a5eeda8328 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" @@ -301,7 +299,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 @@ -533,7 +537,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") @@ -1303,7 +1307,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") } @@ -1315,22 +1319,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 13791378db0..d7a20c94a9e 100644 --- a/pkg/controller/clusterclaim/clusterclaim_controller.go +++ b/pkg/controller/clusterclaim/clusterclaim_controller.go @@ -549,7 +549,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 4b932c4e564..2f46f33d3eb 100644 --- a/pkg/controller/clusterdeployment/clusterdeployment_controller.go +++ b/pkg/controller/clusterdeployment/clusterdeployment_controller.go @@ -456,7 +456,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 } @@ -658,6 +665,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 b99070ec14f..fecc5b62bbc 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 0d819224d61..e86850adc53 100644 --- a/pkg/controller/clusterdeployment/clusterprovisions.go +++ b/pkg/controller/clusterdeployment/clusterprovisions.go @@ -423,7 +423,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 diff --git a/pkg/controller/utils/secrets.go b/pkg/controller/utils/secrets.go index 45bd125eda8..8a520e1b802 100644 --- a/pkg/controller/utils/secrets.go +++ b/pkg/controller/utils/secrets.go @@ -2,23 +2,76 @@ package utils import ( "context" + "errors" "fmt" + "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 } diff --git a/pkg/remoteclient/kubeconfig.go b/pkg/remoteclient/kubeconfig.go index e14061d72fa..205d438d61f 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" "github.com/openshift/hive/pkg/util/scheme" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/discovery" @@ -81,5 +82,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 6139e666c53..852e3564d99 100644 --- a/pkg/remoteclient/remoteclient.go +++ b/pkg/remoteclient/remoteclient.go @@ -18,11 +18,9 @@ import ( kubeclient "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" - "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/client" hivev1 "github.com/openshift/hive/apis/hive/v1" - "github.com/openshift/hive/pkg/constants" "github.com/openshift/hive/pkg/controller/utils" "github.com/openshift/hive/pkg/util/scheme" ) @@ -291,23 +289,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) }