Merge branch 'main' into EVEREST-793-upgrade

.golangci.yml

@@ -48,6 +48,7 @@ linters:
     - wsl              # too many empty lines makes methods too long
     - wrapcheck        # forces to wrap errors everywhere
     - lll              # Just useless in the most cases
+    - perfsprint       # to keep errors consistent
 
 
 issues:

cli-tests/playwright.config.ts

@@ -31,7 +31,7 @@ export default defineConfig({
   forbidOnly: !!process.env.CI,
   /* Retry on CI only */
   retries: process.env.CI ? 2 : 0,
-  timeout: 300_000,
+  timeout: 600_000,
   /* Opt out of parallel tests on CI. */
   workers: process.env.CI ? 1 : undefined,
   /* Reporter to use. See https://playwright.dev/docs/test-reporters */

cli-tests/tests/flow/all-operators.spec.ts

@@ -98,11 +98,14 @@ test.describe('Everest CLI install', async () => {
       );
 
       await out.assertSuccess();
-      // check that the deployment does not exist
-      out = await cli.exec('kubectl get deploy percona-everest -n everest-system');
+      // check that the namespace does not exist
+      out = await cli.exec('kubectl get ns everest-system everest-monitoring everest-olm everest-all');
 
       await out.outErrContainsNormalizedMany([
-        'Error from server (NotFound): deployments.apps "percona-everest" not found',
+		'Error from server (NotFound): namespaces "everest-system" not found',
+		'Error from server (NotFound): namespaces "everest-monitoring" not found',
+		'Error from server (NotFound): namespaces "everest-olm" not found',
+		'Error from server (NotFound): namespaces "everest-all" not found',
       ]);
 
     });

commands/install.go

@@ -29,7 +29,13 @@ import (
 
 func newInstallCmd(l *zap.SugaredLogger) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "install",
+		Use: "install",
+		// The command expects no arguments. So to prevent users from misspelling and confusion
+		// in cases with unexpected spaces like
+		//       ./everestctl install --namespaces=aaa, a
+		// it will return
+		//        Error: unknown command "a" for "everestctl install"
+		Args:    cobra.NoArgs,
 		Example: "everestctl install --namespaces dev,staging,prod --operator.mongodb=true --operator.postgresql=true --operator.xtradb-cluster=true --skip-wizard",
 		Run: func(cmd *cobra.Command, args []string) {
 			initInstallViperFlags(cmd)

commands/upgrade.go

@@ -31,6 +31,12 @@ import (
 func newUpgradeCmd(l *zap.SugaredLogger) *cobra.Command {
 	cmd := &cobra.Command{
 		Use: "upgrade",
+		// The command expects no arguments. So to prevent users from misspelling and confusion
+		// in cases with unexpected spaces like
+		//       ./everestctl upgrade --namespaces=aaa, a
+		// it will return
+		//        Error: unknown command "a" for "everestctl upgrade"
+		Args: cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
 			initUpgradeViperFlags(cmd)
 

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/hashicorp/go-version v1.6.0
 	github.com/operator-framework/api v0.22.0
 	github.com/operator-framework/operator-lifecycle-manager v0.26.0
-	github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284
+	github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818
 	github.com/percona/percona-everest-backend v0.7.0
 	github.com/spf13/cobra v1.8.0
 	github.com/spf13/viper v1.18.2

go.sum

@@ -536,8 +536,8 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9
 github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE=
 github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4=
 github.com/pelletier/go-toml/v2 v2.1.0/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
-github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284 h1:5LKWEvGtaimDPVI3Rj9tFxO+g/zAjUJq0yFIzznQGfc=
-github.com/percona/everest-operator v0.6.0-dev1.0.20240214112044-8f2dea595284/go.mod h1:45pGpvWrPy495qiQqxNuOJor4wif+vTTTJP4Qee8qZk=
+github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818 h1:w4E4zlSTRQQk2/tFAFO5WGquvKRg2ocw7hxcbRjUT58=
+github.com/percona/everest-operator v0.6.0-dev1.0.20240220114053-fae6111d9818/go.mod h1:45pGpvWrPy495qiQqxNuOJor4wif+vTTTJP4Qee8qZk=
 github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901 h1:BDgsZRCjEuxl2/z4yWBqB0s8d20shuIDks7/RVdZiLs=
 github.com/percona/percona-backup-mongodb v1.8.1-0.20230920143330-3b1c2e263901/go.mod h1:fZRCMpUqkWlLVdRKqqaj001LoVP2eo6F0ZhoMPeXDng=
 github.com/percona/percona-everest-backend v0.7.0 h1:ku03G3p1sttWL9rfzeOhUcSarREnszmeWScTqGMynzk=

install.sh

@@ -32,8 +32,6 @@ else
 	echo "KUBECONFIG is not set. Using default k8s cluster"
 fi
 
-echo "Provisioning Everest with monitoring disabled"
-echo "If you want to enable monitoring please refer to the everest installation documentation."
 echo ""
 ./everestctl install --namespaces everest --operator.mongodb=true --operator.postgresql=true --operator.xtradb-cluster=true --skip-wizard
 

pkg/install/install.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"regexp"
 	"strings"
 
 	"github.com/AlecAivazis/survey/v2"
@@ -88,11 +89,29 @@ const (
 	disableTelemetryEnvVar = "DISABLE_TELEMETRY"
 )
 
+//nolint:gochecknoglobals
+var (
+	// ErrNSEmpty appears when the provided list of the namespaces is considered empty.
+	ErrNSEmpty = errors.New("namespace list is empty. Specify at least one namespace")
+	// ErrNSReserved appears when some of the provided names are forbidden to use.
+	ErrNSReserved = func(ns string) error {
+		return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns)
+	}
+	// ErrNameNotRFC1035Compatible appears when some of the provided names are not RFC1035 compatible.
+	ErrNameNotRFC1035Compatible = func(fieldName string) error {
+		return fmt.Errorf(`'%s' is not RFC 1035 compatible. The name should contain only lowercase alphanumeric characters or '-', start with an alphabetic character, end with an alphanumeric character`,
+			fieldName,
+		)
+	}
+)
+
 type (
 	// Config stores configuration for the operators.
 	Config struct {
-		// Namespaces defines comma-separated list of namespaces that everest can operate in.
+		// Namespaces is a user-defined string represents raw non-validated comma-separated list of namespaces for everest to operate in.
 		Namespaces string `mapstructure:"namespaces"`
+		// NamespacesList validated list of namespaces that everest can operate in.
+		NamespacesList []string `mapstructure:"namespaces-map"`
 		// SkipWizard skips wizard during installation.
 		SkipWizard bool `mapstructure:"skip-wizard"`
 		// KubeconfigPath is a path to a kubeconfig
@@ -114,11 +133,6 @@ type (
 	}
 )
 
-// NamespacesList returns list of the namespaces that everest can operate in.
-func (c Config) NamespacesList() []string {
-	return strings.Split(c.Namespaces, ",")
-}
-
 // NewInstall returns a new Install struct.
 func NewInstall(c Config, l *zap.SugaredLogger) (*Install, error) {
 	cli := &Install{
@@ -205,15 +219,11 @@ func (o *Install) populateConfig() error {
 		}
 	}
 
-	if len(o.config.NamespacesList()) == 0 {
-		return errors.New("namespace list is empty. Specify the comma-separated list of namespaces using the --namespaces flag, at least one namespace is required")
-	}
-
-	for _, ns := range o.config.NamespacesList() {
-		if ns == SystemNamespace || ns == MonitoringNamespace {
-			return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns)
-		}
+	l, err := ValidateNamespaces(o.config.Namespaces)
+	if err != nil {
+		return err
 	}
+	o.config.NamespacesList = l
 
 	return nil
 }
@@ -295,7 +305,7 @@ func (o *Install) provisionEverestOperator(ctx context.Context, recVer *version.
 	}
 
 	o.l.Info("Creating operator group for everest")
-	if err := o.kubeClient.CreateOperatorGroup(ctx, systemOperatorGroup, SystemNamespace, o.config.NamespacesList()); err != nil {
+	if err := o.kubeClient.CreateOperatorGroup(ctx, systemOperatorGroup, SystemNamespace, o.config.NamespacesList); err != nil {
 		return err
 	}
 
@@ -337,15 +347,15 @@ func (o *Install) provisionEverest(ctx context.Context, v *goversion.Version) er
 	}
 
 	o.l.Info("Updating cluster role bindings for everest-admin")
-	if err := o.kubeClient.UpdateClusterRoleBinding(ctx, everestServiceAccountClusterRoleBinding, o.config.NamespacesList()); err != nil {
+	if err := o.kubeClient.UpdateClusterRoleBinding(ctx, everestServiceAccountClusterRoleBinding, o.config.NamespacesList); err != nil {
 		return err
 	}
 
 	return nil
 }
 
 func (o *Install) provisionDBNamespaces(ctx context.Context, recVer *version.RecommendedVersion) error {
-	for _, namespace := range o.config.NamespacesList() {
+	for _, namespace := range o.config.NamespacesList {
 		namespace := namespace
 		if err := o.createNamespace(namespace); err != nil {
 			return err
@@ -399,27 +409,12 @@ func (o *Install) runEverestWizard() error {
 		return err
 	}
 
-	nsList := strings.Split(namespaces, ",")
-	for _, ns := range nsList {
-		ns = strings.TrimSpace(ns)
-		if ns == "" {
-			continue
-		}
-
-		if ns == SystemNamespace {
-			return fmt.Errorf("'%s' namespace is reserved for Everest internals. Please specify another namespace", ns)
-		}
-
-		if o.config.Namespaces != "" {
-			o.config.Namespaces += ","
-		}
-
-		o.config.Namespaces += ns
-	}
-
-	if len(o.config.NamespacesList()) == 0 {
-		return errors.New("namespace list is empty. Specify at least one namespace")
+	list, err := ValidateNamespaces(namespaces)
+	if err != nil {
+		return err
 	}
+	o.config.Namespaces = namespaces
+	o.config.NamespacesList = list
 
 	return nil
 }
@@ -577,15 +572,15 @@ func (o *Install) installOperator(ctx context.Context, channel, operatorName, na
 			},
 		}
 		if operatorName == everestOperatorName {
-			params.TargetNamespaces = o.config.NamespacesList()
+			params.TargetNamespaces = o.config.NamespacesList
 			params.SubscriptionConfig.Env = append(params.SubscriptionConfig.Env, []corev1.EnvVar{
 				{
 					Name:  EverestMonitoringNamespaceEnvVar,
 					Value: MonitoringNamespace,
 				},
 				{
 					Name:  kubernetes.EverestDBNamespacesEnvVar,
-					Value: o.config.Namespaces,
+					Value: strings.Join(o.config.NamespacesList, ","),
 				},
 			}...)
 		}
@@ -671,3 +666,46 @@ func (o *Install) generateToken(ctx context.Context) (*token.ResetResponse, erro
 
 	return res, nil
 }
+
+// ValidateNamespaces validates a comma-separated namespaces string.
+func ValidateNamespaces(str string) ([]string, error) {
+	nsList := strings.Split(str, ",")
+	m := make(map[string]struct{})
+	for _, ns := range nsList {
+		ns = strings.TrimSpace(ns)
+		if ns == "" {
+			continue
+		}
+
+		if ns == SystemNamespace || ns == MonitoringNamespace {
+			return nil, ErrNSReserved(ns)
+		}
+
+		if err := validateRFC1035(ns); err != nil {
+			return nil, err
+		}
+
+		m[ns] = struct{}{}
+	}
+
+	list := make([]string, 0, len(m))
+	for k := range m {
+		list = append(list, k)
+	}
+
+	if len(list) == 0 {
+		return nil, ErrNSEmpty
+	}
+	return list, nil
+}
+
+// validates names to be RFC-1035 compatible  https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names
+func validateRFC1035(s string) error {
+	rfc1035Regex := "^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$"
+	re := regexp.MustCompile(rfc1035Regex)
+	if !re.MatchString(s) {
+		return ErrNameNotRFC1035Compatible(s)
+	}
+
+	return nil
+}

pkg/install/install_test.go

@@ -0,0 +1,98 @@
+package install
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateNamespaces(t *testing.T) {
+	t.Parallel()
+
+	type tcase struct {
+		name   string
+		input  string
+		output []string
+		error  error
+	}
+
+	tcases := []tcase{
+		{
+			name:   "empty string",
+			input:  "",
+			output: nil,
+			error:  ErrNSEmpty,
+		},
+		{
+			name:   "several empty strings",
+			input:  "   ,   ,",
+			output: nil,
+			error:  ErrNSEmpty,
+		},
+		{
+			name:   "correct",
+			input:  "aaa,bbb,ccc",
+			output: []string{"aaa", "bbb", "ccc"},
+			error:  nil,
+		},
+		{
+			name: "correct with spaces",
+			input: `    aaa, bbb 
+,ccc   `,
+			output: []string{"aaa", "bbb", "ccc"},
+			error:  nil,
+		},
+		{
+			name:   "reserved system ns",
+			input:  "everest-system",
+			output: nil,
+			error:  ErrNSReserved("everest-system"),
+		},
+		{
+			name:   "reserved system ns and empty ns",
+			input:  "everest-system,    ",
+			output: nil,
+			error:  ErrNSReserved("everest-system"),
+		},
+		{
+			name:   "reserved monitoring ns",
+			input:  "everest-monitoring",
+			output: nil,
+			error:  ErrNSReserved("everest-monitoring"),
+		},
+		{
+			name:   "duplicated ns",
+			input:  "aaa,bbb,aaa",
+			output: []string{"aaa", "bbb"},
+			error:  nil,
+		},
+		{
+			name:   "name is too long",
+			input:  "e1234567890123456789012345678901234567890123456789012345678901234567890,bbb",
+			output: nil,
+			error:  ErrNameNotRFC1035Compatible("e1234567890123456789012345678901234567890123456789012345678901234567890"),
+		},
+		{
+			name:   "name starts with number",
+			input:  "1aaa,bbb",
+			output: nil,
+			error:  ErrNameNotRFC1035Compatible("1aaa"),
+		},
+		{
+			name:   "name contains special characters",
+			input:  "aa12a,b$s",
+			output: nil,
+			error:  ErrNameNotRFC1035Compatible("b$s"),
+		},
+	}
+
+	for _, tc := range tcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			output, err := ValidateNamespaces(tc.input)
+			assert.Equal(t, tc.error, err)
+			assert.ElementsMatch(t, tc.output, output)
+		})
+	}
+}