[Project] Filter invalid labels if creating project from leader reque…

…st (#3168)

pkg/common/k8s.go

@@ -166,11 +166,12 @@ func getKubeconfigFromHomeDir() string {
  return ""
 }
 
-func ValidateNodeSelector(nodeSelector map[string]string) error {
- if nodeSelector == nil {
+// ValidateLabels validates the given labels according to k8s label constraints
+func ValidateLabels(labels map[string]string) error {
+ if labels == nil {
  return nil
  }
- for labelKey, labelValue := range nodeSelector {
+ for labelKey, labelValue := range labels {
  if errs := validation.IsValidLabelValue(labelValue); len(errs) > 0 {
  errs = append([]string{fmt.Sprintf("Invalid value: %s", labelValue)}, errs...)
  return nuclio.NewErrBadRequest(strings.Join(errs, ", "))
@@ -186,3 +187,20 @@ func ValidateNodeSelector(nodeSelector map[string]string) error {
  }
  return nil
 }
+
+// FilterInvalidLabels filters out invalid kubernetes labels from a map of labels
+func FilterInvalidLabels(labels map[string]string) map[string]string {
+
+ // From k8s docs:
+ // a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.',
+ // and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345',
+ // regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')
+ filteredLabels := map[string]string{}
+ for key, value := range labels {
+ if len(validation.IsQualifiedName(key)) != 0 || len(validation.IsValidLabelValue(value)) != 0 {
+ continue
+ }
+ filteredLabels[key] = value
+ }
+ return filteredLabels
+}

pkg/common/k8s_test.go

@@ -0,0 +1,61 @@
+//go:build test_unit
+
+/*
+Copyright 2023 The Nuclio Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/suite"
+)
+
+type k8sTestSuite struct {
+ suite.Suite
+}
+
+func (suite *k8sTestSuite) TestFilterInvalidLabels() {
+ invalidLabels := map[string]string{
+ "my@weird/label": "value",
+ "my.wierd/label": "value@",
+ "%weird+/label": "v8$alue",
+ "email.like@label": "value",
+ }
+
+ labels := map[string]string{
+ "valid": "label",
+ "another-valid": "label-value",
+ "also_123_valid": "label_456_value",
+ }
+
+ // add invalid labels to labels
+ for key, value := range invalidLabels {
+ labels[key] = value
+ }
+
+ filteredLabels := FilterInvalidLabels(labels)
+
+ suite.Require().Equal(len(filteredLabels), len(labels)-len(invalidLabels))
+ for key := range invalidLabels {
+ _, ok := filteredLabels[key]
+ suite.Require().False(ok, "invalid label %s should not be in filtered labels", key)
+ }
+}
+
+func TestK8sTestSuite(t *testing.T) {
+ suite.Run(t, new(k8sTestSuite))
+}

pkg/platform/abstract/platform.go

@@ -450,7 +450,8 @@ func (ap *Platform) ValidateFunctionConfig(ctx context.Context, functionConfig *
  return errors.Wrap(err, "Min max replicas validation failed")
  }
 
- if err := common.ValidateNodeSelector(functionConfig.Spec.NodeSelector); err != nil {
+ // validate function node selector
+ if err := common.ValidateLabels(functionConfig.Spec.NodeSelector); err != nil {
  return errors.Wrap(err, "Node selector validation failed")
  }
 
@@ -798,6 +799,13 @@ func (ap *Platform) EnrichCreateProjectConfig(createProjectOptions *platform.Cre
  createProjectOptions.ProjectConfig.Spec.Owner = createProjectOptions.AuthSession.GetUsername()
  }
 
+ if ap.Config.ProjectsLeader != nil && createProjectOptions.RequestOrigin == ap.Config.ProjectsLeader.Kind {
+
+ // to align with the leaders (that allow invalid k8s labels), we just ignore the project's invalid labels
+ // instead of failing validation later on
+ createProjectOptions.ProjectConfig.Meta.Labels = common.FilterInvalidLabels(createProjectOptions.ProjectConfig.Meta.Labels)
+ }
+
  return nil
 }
 
@@ -808,8 +816,14 @@ func (ap *Platform) ValidateProjectConfig(projectConfig *platform.ProjectConfig)
  return nuclio.NewErrBadRequest("Project name cannot be empty")
  }
 
- if err := common.ValidateNodeSelector(projectConfig.Spec.DefaultFunctionNodeSelector); err != nil {
- return nuclio.WrapErrBadRequest(err)
+ // validate project labels
+ if err := common.ValidateLabels(projectConfig.Meta.Labels); err != nil {
+ return errors.Wrap(err, "Project labels validation failed")
+ }
+
+ // validate default node selector
+ if err := common.ValidateLabels(projectConfig.Spec.DefaultFunctionNodeSelector); err != nil {
+ return errors.Wrap(err, "Default function node selector validation failed")
  }
 
  // project name should adhere Kubernetes label restrictions

pkg/platform/abstract/platform_test.go

@@ -180,6 +180,21 @@ func (suite *AbstractPlatformTestSuite) TestProjectCreateOptions() {
  },
  ExpectValidationFailure: true,
  },
+ {
+ Name: "InvalidLabels",
+ CreateProjectOptions: &platform.CreateProjectOptions{
+ ProjectConfig: &platform.ProjectConfig{
+ Meta: platform.ProjectMeta{
+ Name: "a-name",
+ Labels: map[string]string{
+ "invalid label ## .. %%": "value",
+ "valid-key": "invalid value ## .. %%",
+ },
+ },
+ },
+ },
+ ExpectValidationFailure: true,
+ },
  } {
  suite.Run(testCase.Name, func() {
  defer func() {

pkg/platform/abstract/project/external/leader/iguazio/client.go

@@ -139,7 +139,7 @@ func (c *Client) Create(ctx context.Context, createProjectOptions *platform.Crea
 
  // send the request
  c.logger.DebugWithCtx(ctx,
- "Creating project",
+ "Creating project request to leader",
  "body", string(body))
  responseBody, response, err := common.SendHTTPRequestWithContext(ctx,
  c.httpClient,

pkg/platform/abstract/project/external/leader/iguazio/synchronizer.go

@@ -19,9 +19,9 @@ package iguazio
 import (
  "context"
  "fmt"
- "regexp"
  "time"
 
+ "github.com/nuclio/nuclio/pkg/common"
  "github.com/nuclio/nuclio/pkg/errgroup"
  "github.com/nuclio/nuclio/pkg/platform"
  "github.com/nuclio/nuclio/pkg/platform/abstract/project"
@@ -206,7 +206,7 @@ func (c *Synchronizer) synchronizeProjectsFromLeader(ctx context.Context,
  createProjectErrGroup.Go("create projects", func() error {
 
  // filter out labels that are not allowed by kubernetes
- projectInstance.Meta.Labels = c.filterInvalidLabels(projectInstance.Meta.Labels)
+ projectInstance.Meta.Labels = common.FilterInvalidLabels(projectInstance.Meta.Labels)
 
  c.logger.DebugWithCtx(ctx, "Creating project from leader sync", "projectInstance", *projectInstance)
  createProjectConfig := &platform.CreateProjectOptions{
@@ -241,7 +241,7 @@ func (c *Synchronizer) synchronizeProjectsFromLeader(ctx context.Context,
  updateProjectErrGroup.Go("update projects", func() error {
 
  // filter out labels that are not allowed by kubernetes
- projectInstance.Meta.Labels = c.filterInvalidLabels(projectInstance.Meta.Labels)
+ projectInstance.Meta.Labels = common.FilterInvalidLabels(projectInstance.Meta.Labels)
 
  c.logger.DebugWith("Updating project from leader sync", "projectInstance", *projectInstance)
  updateProjectOptions := &platform.UpdateProjectOptions{
@@ -276,19 +276,3 @@ func (c *Synchronizer) synchronizeProjectsFromLeader(ctx context.Context,
 func (c *Synchronizer) generateUniqueProjectKey(configInstance *platform.ProjectConfig) string {
  return fmt.Sprintf("%s:%s", configInstance.Meta.Namespace, configInstance.Meta.Name)
 }
-
-func (c *Synchronizer) filterInvalidLabels(labels map[string]string) map[string]string {
-
- // From k8s docs:
- // a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.',
- // and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345',
- // regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')
- regex := regexp.MustCompile(`^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$`)
- filteredLabels := map[string]string{}
- for key, value := range labels {
- if regex.MatchString(key) {
- filteredLabels[key] = value
- }
- }
- return filteredLabels
-}

pkg/platform/abstract/project/external/leader/iguazio/synchronizer_test.go

@@ -191,33 +191,6 @@ func (suite *SynchronizerTestSuite) TestLeaderProjectsThatExistInternally() {
  &testBeginningTime)
 }
 
-func (suite *SynchronizerTestSuite) TestFilterInvalidLabels() {
- invalidLabels := map[string]string{
- "my@weird/label": "value",
- "my.wierd/label": "value@",
- "%weird+/label": "v8$alue",
- }
-
- labels := map[string]string{
- "valid": "label",
- "another-valid": "label-value",
- "also_123_valid": "label_456_value",
- }
-
- // add invalid labels to labels
- for key, value := range invalidLabels {
- labels[key] = value
- }
-
- filteredLabels := suite.synchronizer.filterInvalidLabels(labels)
-
- suite.Require().Equal(len(filteredLabels), len(labels)-len(invalidLabels))
- for key := range invalidLabels {
- _, ok := filteredLabels[key]
- suite.Require().False(ok, "invalid label %s should not be in filtered labels", key)
- }
-}
-
 func (suite *SynchronizerTestSuite) testSynchronizeProjectsFromLeader(namespace string,
  leaderProjects []platform.Project,
  internalProjects []platform.Project,

pkg/platform/kube/test/platform_test.go

@@ -1597,6 +1597,7 @@ func (suite *DeployFunctionTestSuite) TestDeployFromGitSanity() {
  return true
  })
 }
+
 func (suite *DeployFunctionTestSuite) createPlatformConfigmapWithJSONLogger() *v1.ConfigMap {
 
  // create a platform config configmap with a json logger sink (this is how it is on production)
@@ -2087,6 +2088,55 @@ func (suite *ProjectTestSuite) TestCreate() {
  suite.Require().Equal(projectConfig, *createdProject.GetConfig())
 }
 
+func (suite *ProjectTestSuite) TestCreateFromLeaderIgnoreInvalidLabels() {
+ invalidLabelKey1 := "invalid.label@-key"
+ invalidLabelKey2 := "the-key-is-invalid"
+ projectConfig := platform.ProjectConfig{
+ Meta: platform.ProjectMeta{
+ Name: "test-project",
+ Namespace: suite.Namespace,
+ Labels: map[string]string{
+ invalidLabelKey1: "label-value",
+ invalidLabelKey2: "inva!id_v8$alue",
+ },
+ },
+ Spec: platform.ProjectSpec{
+ Description: "some description",
+ },
+ }
+
+ // set the platform's leader kind
+ suite.Platform.GetConfig().ProjectsLeader = &platformconfig.ProjectsLeader{
+ Kind: platformconfig.ProjectsLeaderKindMock,
+ }
+
+ // create project
+ err := suite.Platform.CreateProject(suite.Ctx, &platform.CreateProjectOptions{
+ ProjectConfig: &projectConfig,
+ RequestOrigin: platformconfig.ProjectsLeaderKindMock,
+ })
+ suite.Require().NoError(err, "Failed to create project")
+ defer func() {
+ err = suite.Platform.DeleteProject(suite.Ctx, &platform.DeleteProjectOptions{
+ Meta: projectConfig.Meta,
+ Strategy: platform.DeleteProjectStrategyRestricted,
+ })
+ suite.Require().NoError(err, "Failed to delete project")
+ }()
+
+ // get created project
+ projects, err := suite.Platform.GetProjects(suite.Ctx, &platform.GetProjectsOptions{
+ Meta: projectConfig.Meta,
+ })
+ suite.Require().NoError(err, "Failed to get projects")
+ suite.Require().Equal(len(projects), 1)
+
+ // verify created project does not contain the invalid label
+ createdProject := projects[0]
+ suite.Require().NotContains(createdProject.GetConfig().Meta.Labels, invalidLabelKey1)
+ suite.Require().NotContains(createdProject.GetConfig().Meta.Labels, invalidLabelKey2)
+}
+
 func (suite *ProjectTestSuite) TestUpdate() {
  projectConfig := platform.ProjectConfig{
  Meta: platform.ProjectMeta{