From e6cae6d1ef1858112e9b87583f98017c77b2eaba Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Thu, 2 Mar 2023 16:12:14 +0100 Subject: [PATCH] Refactor RecoverData to rely on WriteInfoToFile Signed-off-by: Stephen Kitt --- cmd/subctl/deploybroker.go | 22 ++++++++++++- pkg/broker/file.go | 52 +++++++++---------------------- pkg/broker/ipsec_psk.go | 15 +++------ pkg/broker/recover_broker_info.go | 33 ++------------------ 4 files changed, 44 insertions(+), 78 deletions(-) diff --git a/cmd/subctl/deploybroker.go b/cmd/subctl/deploybroker.go index 1aa2f82c0..bca1012fb 100644 --- a/cmd/subctl/deploybroker.go +++ b/cmd/subctl/deploybroker.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/submariner-io/admiral/pkg/reporter" "github.com/submariner-io/subctl/internal/cli" @@ -90,7 +91,26 @@ func deployBrokerInContext(clusterInfo *cluster.Info, namespace string, status r return err //nolint:wrapcheck // No need to wrap errors here. } + ipsecPSK := []byte{} + var err error + + if ipsecSubmFile != "" { + ipsecData, err := broker.ReadInfoFromFile(ipsecSubmFile) + if err != nil { + return errors.Wrapf(err, "error importing IPsec PSK from file %q", ipsecSubmFile) + } + + ipsecPSK = ipsecData.IPSecPSK.Data["psk"] + } + + if len(ipsecPSK) == 0 { + ipsecPSK, err = broker.GenerateRandomPSK() + if err != nil { + return err //nolint:wrapcheck // No need to wrap errors here. + } + } + return broker.WriteInfoToFile( //nolint:wrapcheck // No need to wrap errors here. - clusterInfo.RestConfig, namespace, ipsecSubmFile, + clusterInfo.RestConfig, namespace, ipsecPSK, sets.New(deployflags.BrokerSpec.Components...), deployflags.BrokerSpec.DefaultCustomDomains, status) } diff --git a/pkg/broker/file.go b/pkg/broker/file.go index ab09d4bb8..bf3b45bdd 100644 --- a/pkg/broker/file.go +++ b/pkg/broker/file.go @@ -38,33 +38,36 @@ import ( const InfoFileName = "broker-info.subm" -func WriteInfoToFile(restConfig *rest.Config, brokerNamespace, ipsecFile string, components sets.Set[string], +func WriteInfoToFile(restConfig *rest.Config, brokerNamespace string, ipsecPSK []byte, components sets.Set[string], customDomains []string, status reporter.Interface, ) error { status.Start("Saving broker info to file %q", InfoFileName) defer status.End() - kubeClient, err := kubernetes.NewForConfig(restConfig) + newFilename, err := backupIfExists(InfoFileName) if err != nil { - return status.Error(err, "error creating Kubernetes client") + return status.Error(err, "error backing up the broker file") } - data, err := newDataFrom(kubeClient, brokerNamespace, ipsecFile) + if newFilename != "" { + status.Success("Backed up previous file %q to %q", InfoFileName, newFilename) + } + + kubeClient, err := kubernetes.NewForConfig(restConfig) if err != nil { - // TODO return reporter.Error(err, "error initializing broker info") - return err + return status.Error(err, "error creating Kubernetes client") } - data.BrokerURL = restConfig.Host + restConfig.APIPath + data := &Info{} - newFilename, err := backupIfExists(InfoFileName) + data.ClientToken, err = rbac.GetClientTokenSecret(context.TODO(), kubeClient, brokerNamespace, constants.SubmarinerBrokerAdminSA) if err != nil { - return status.Error(err, "error backing up the broker file") + return errors.Wrap(err, "error getting broker client secret") } - if newFilename != "" { - status.Success("Backed up previous file %q to %q", InfoFileName, newFilename) - } + data.IPSecPSK = wrapIPSecPSKSecret(ipsecPSK) + + data.BrokerURL = restConfig.Host + restConfig.APIPath data.ServiceDiscovery = components.Has(component.ServiceDiscovery) data.Components = components.UnsortedList() @@ -92,31 +95,6 @@ func ReadInfoFromFile(filename string) (*Info, error) { return data, errors.Wrap(json.Unmarshal(bytes, data), "error unmarshalling data") } -func newDataFrom(kubeClient kubernetes.Interface, brokerNamespace, ipsecFile string) (*Info, error) { - var err error - data := &Info{} - - data.ClientToken, err = rbac.GetClientTokenSecret(context.TODO(), kubeClient, brokerNamespace, constants.SubmarinerBrokerAdminSA) - if err != nil { - return nil, errors.Wrap(err, "error getting broker client secret") - } - - if ipsecFile != "" { - ipsecData, err := ReadInfoFromFile(ipsecFile) - if err != nil { - return nil, errors.Wrapf(err, "error importing IPsec PSK from file %q", ipsecFile) - } - - data.IPSecPSK = ipsecData.IPSecPSK - - return data, err - } - - data.IPSecPSK, err = newIPSECPSKSecret() - - return data, err -} - func backupIfExists(fileName string) (string, error) { if _, err := os.Stat(fileName); os.IsNotExist(err) { return "", nil diff --git a/pkg/broker/ipsec_psk.go b/pkg/broker/ipsec_psk.go index b30c9ddf6..4112143c8 100644 --- a/pkg/broker/ipsec_psk.go +++ b/pkg/broker/ipsec_psk.go @@ -30,20 +30,15 @@ const ( ipsecSecretLength = 48 ) -// generateRandomPSK returns securely generated n-byte array. -func generateRandomPSK(n int) ([]byte, error) { - psk := make([]byte, n) +// GenerateRandomPSK returns a securely generated array suitable for use as a PSK. +func GenerateRandomPSK() ([]byte, error) { + psk := make([]byte, ipsecSecretLength) _, err := rand.Read(psk) return psk, err //nolint:wrapcheck // No need to wrap here } -func newIPSECPSKSecret() (*v1.Secret, error) { - psk, err := generateRandomPSK(ipsecSecretLength) - if err != nil { - return nil, err - } - +func wrapIPSecPSKSecret(psk []byte) *v1.Secret { pskSecretData := make(map[string][]byte) pskSecretData["psk"] = psk @@ -54,5 +49,5 @@ func newIPSECPSKSecret() (*v1.Secret, error) { Data: pskSecretData, } - return pskSecret, nil + return pskSecret } diff --git a/pkg/broker/recover_broker_info.go b/pkg/broker/recover_broker_info.go index 55d8a6978..a6b8149d0 100644 --- a/pkg/broker/recover_broker_info.go +++ b/pkg/broker/recover_broker_info.go @@ -19,16 +19,12 @@ limitations under the License. package broker import ( - "context" "encoding/base64" "github.com/submariner-io/admiral/pkg/reporter" - "github.com/submariner-io/subctl/internal/constants" - "github.com/submariner-io/subctl/internal/rbac" "github.com/submariner-io/subctl/pkg/cluster" "github.com/submariner-io/submariner-operator/api/v1alpha1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" ) func RecoverData( @@ -37,23 +33,6 @@ func RecoverData( status.Start("Retrieving data to reconstruct broker-info.subm") defer status.End() - data := &Info{} - var err error - - data.BrokerURL = brokerCluster.RestConfig.Host + brokerCluster.RestConfig.APIPath - - data.ClientToken, err = rbac.GetClientTokenSecret( - context.TODO(), brokerCluster.ClientProducer.ForKubernetes(), namespace, - constants.SubmarinerBrokerAdminSA, - ) - if err != nil { - return status.Error(err, "error getting broker client secret") - } - - data.Components = broker.Spec.Components - data.ServiceDiscovery = data.IsServiceDiscoveryEnabled() - data.CustomDomains = &broker.Spec.DefaultCustomDomains - status.Success("Retrieving IPSec PSK secret from Submariner found on cluster %s", submCluster.Name) decodedPSKSecret, err := base64.StdEncoding.DecodeString(submCluster.Submariner.Spec.CeIPSecPSK) @@ -61,16 +40,10 @@ func RecoverData( return status.Error(err, "error decoding the secret") } - data.IPSecPSK = &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: ipsecPSKSecretName, - }, - Data: map[string][]byte{"psk": decodedPSKSecret}, - } - status.Success("Successfully retrieved the data. Writing it to broker-info.subm") - err = data.writeToFile("broker-info.subm") + err = WriteInfoToFile(brokerCluster.RestConfig, namespace, decodedPSKSecret, + sets.New(broker.Spec.Components...), broker.Spec.DefaultCustomDomains, status) return status.Error(err, "error reconstructing broker-info.subm") }