Merge pull request #72 from portainer/fix68-namespace-validation

fix: update namespace creation and deletion logic
portainer · Oct 11, 2023 · 05661e0 · 05661e0
2 parents f2c3906 + 2f0a24d
commit 05661e0
Show file tree

Hide file tree

Showing 12 changed files with 21 additions and 24 deletions.
diff --git a/internal/adapter/adapter.go b/internal/adapter/adapter.go
@@ -44,6 +44,8 @@ type (
 	// - Server Configuration: Contains configuration related to the k2d server, which is used when
 	//   creating certain resources.
 	//
+	// - Namespace deletion delay: Contains the delay that k2d waits after a namespace is deleted.
+	//
 	// This struct is a comprehensive utility for managing the interactions between Docker and Kubernetes.
 	KubeDockerAdapter struct {
 		cli                    *client.Client
@@ -52,6 +54,7 @@ type (
 		conversionScheme       *runtime.Scheme
 		k2dServerConfiguration *types.K2DServerConfiguration
 		logger                 *zap.SugaredLogger
+		namespaceDeletionDelay time.Duration
 		registrySecretStore    store.SecretStore
 		startTime              time.Time
 		secretStore            store.SecretStore
@@ -109,6 +112,7 @@ func NewKubeDockerAdapter(options *KubeDockerAdapterOptions) (*KubeDockerAdapter
 		configMapStore:         configMapStore,
 		k2dServerConfiguration: options.ServerConfiguration,
 		logger:                 options.Logger,
+		namespaceDeletionDelay: options.K2DConfig.OperationNamespaceDeletionDelay,
 		registrySecretStore:    registrySecretStore,
 		secretStore:            secretStore,
 		startTime:              time.Now(),

diff --git a/internal/adapter/namespace.go b/internal/adapter/namespace.go
@@ -78,7 +78,8 @@ func (adapter *KubeDockerAdapter) DeleteNamespace(ctx context.Context, namespace
 
 	// This is just to make sure that the containers have been properly deleted
 	// before we try to delete the network
-	time.Sleep(3 * time.Second)
+	// This is configurable, see OperationNamespaceDeletionDelay in config.go
+	time.Sleep(adapter.namespaceDeletionDelay)
 
 	networkName := naming.BuildNetworkName(namespaceName)
 	err = adapter.cli.NetworkRemove(ctx, networkName)
@@ -90,7 +91,6 @@ func (adapter *KubeDockerAdapter) DeleteNamespace(ctx context.Context, namespace
 }
 
 func (adapter *KubeDockerAdapter) GetNamespace(ctx context.Context, namespaceName string) (*corev1.Namespace, error) {
-
 	networkName := naming.BuildNetworkName(namespaceName)
 
 	network, err := adapter.getNetwork(ctx, networkName)

diff --git a/internal/api/apis/apps/deployments/deployments.go b/internal/api/apis/apps/deployments/deployments.go
@@ -49,7 +49,6 @@ func (svc DeploymentService) RegisterDeploymentAPI(ws *restful.WebService) {
 		Param(ws.PathParameter("name", "name of the deployment").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/deployments/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeleteDeployment).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the deployment").DataType("string")))

diff --git a/internal/api/core/v1/configmaps/configmaps.go b/internal/api/core/v1/configmaps/configmaps.go
@@ -49,7 +49,6 @@ func (svc ConfigMapService) RegisterConfigMapAPI(ws *restful.WebService) {
 		Param(ws.PathParameter("name", "name of the configmap").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/configmaps/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeleteConfigMap).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the configmap").DataType("string")))

diff --git a/internal/api/core/v1/namespaces/create.go b/internal/api/core/v1/namespaces/create.go
@@ -6,8 +6,6 @@ import (
 
 	"github.com/emicklei/go-restful/v3"
 	"github.com/portainer/k2d/internal/api/utils"
-	"github.com/portainer/k2d/internal/controller"
-	"github.com/portainer/k2d/internal/types"
 	httputils "github.com/portainer/k2d/pkg/http"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -29,7 +27,11 @@ func (svc NamespaceService) CreateNamespace(r *restful.Request, w *restful.Respo
 		return
 	}
 
-	svc.operations <- controller.NewOperation(namespace, controller.HighPriorityOperation, r.HeaderParameter(types.RequestIDHeader))
+	err = svc.adapter.CreateNetworkFromNamespace(r.Request.Context(), namespace)
+	if err != nil {
+		utils.HttpError(r, w, http.StatusInternalServerError, fmt.Errorf("unable to create namespace: %w", err))
+		return
+	}
 
 	namespace.CreationTimestamp = metav1.Now()
 	namespace.UID = uuid.NewUUID()

diff --git a/internal/api/core/v1/persistentvolumeclaims/persistentvolumeclaims.go b/internal/api/core/v1/persistentvolumeclaims/persistentvolumeclaims.go
@@ -59,7 +59,6 @@ func (svc PersistentVolumeClaimService) RegisterPersistentVolumeClaimAPI(ws *res
 		Param(ws.PathParameter("name", "name of the persistentvolumeclaim").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/persistentvolumeclaims/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeletePersistentVolumeClaim).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the persistentvolumeclaim").DataType("string")))

diff --git a/internal/api/core/v1/pods/pods.go b/internal/api/core/v1/pods/pods.go
@@ -49,7 +49,6 @@ func (svc PodService) RegisterPodAPI(ws *restful.WebService) {
 		Param(ws.PathParameter("name", "name of the pod").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/pods/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeletePod).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the pod").DataType("string")))

diff --git a/internal/api/core/v1/secrets/secrets.go b/internal/api/core/v1/secrets/secrets.go
@@ -51,7 +51,6 @@ func (svc SecretService) RegisterSecretAPI(ws *restful.WebService) {
 		Param(ws.PathParameter("name", "name of the secret").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/secrets/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeleteSecret).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the secret").DataType("string")))

diff --git a/internal/api/core/v1/services/services.go b/internal/api/core/v1/services/services.go
@@ -49,7 +49,6 @@ func (svc ServiceService) RegisterServiceAPI(ws *restful.WebService) {
 		Param(ws.PathParameter("name", "name of the service").DataType("string")))
 
 	ws.Route(ws.DELETE("/v1/namespaces/{namespace}/services/{name}").
-		Filter(utils.NamespaceValidation(svc.adapter)).
 		To(svc.DeleteService).
 		Param(ws.PathParameter("namespace", "namespace name").DataType("string")).
 		Param(ws.PathParameter("name", "name of the service").DataType("string")))

diff --git a/internal/api/utils/middleware.go b/internal/api/utils/middleware.go
@@ -21,6 +21,9 @@ import (
 // If the namespace is valid, the function adds it as an attribute to the request object and continues
 // the request processing by invoking the next filter in the chain.
 //
+// Note: This filter is not used by any DELETE endpoints because they are usually called sequentially by
+// Kubernetes clients. The namespace is usually deleted before the validation can be performed.
+//
 // Parameters:
 //   - adapter: A pointer to an initialized KubeDockerAdapter object.
 //

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -36,6 +36,13 @@ type Config struct {
 	// the default value is set to 25.
 	OperationBatchMaxSize int `env:"K2D_OPERATION_BATCH_MAX_SIZE,default=25"`
 
+	// OperationNamespaceDeletionDelay represents the delay that k2d waits after a namespace is deleted.
+	// This delay is used to ensure that all resources associated with the namespace are deleted before
+	// k2d attempts to delete the network from the Docker environment.
+	// If not provided through an environment variable named K2D_OPERATION_NAMESPACE_DELETION_DELAY,
+	// the default value is set to 3 seconds (3s).
+	OperationNamespaceDeletionDelay time.Duration `env:"K2D_OPERATION_NAMESPACE_DELETION_DELAY,default=3s"`
+
 	// Port represents the port number for the application.
 	// If not provided through an environment variable named K2D_PORT,
 	// the default value is set to 6443.

diff --git a/internal/controller/controller.go b/internal/controller/controller.go
@@ -168,14 +168,6 @@ func (controller *OperationController) processPriorityOperations(ops []Operation
 
 func (controller *OperationController) processOperation(op Operation) {
 	switch op.Operation.(type) {
-	case *corev1.Namespace:
-		err := controller.createNamespace(op)
-		if err != nil {
-			controller.logger.Errorw("unable to create namespace",
-				"error", err,
-				"request_id", op.RequestID,
-			)
-		}
 	case *corev1.Pod:
 		err := controller.createPod(op)
 		if err != nil {
@@ -225,11 +217,6 @@ func (controller *OperationController) processOperation(op Operation) {
 	}
 }
 
-func (controller *OperationController) createNamespace(op Operation) error {
-	namespace := op.Operation.(*corev1.Namespace)
-	return controller.adapter.CreateNetworkFromNamespace(context.TODO(), namespace)
-}
-
 func (controller *OperationController) createPod(op Operation) error {
 	pod := op.Operation.(*corev1.Pod)
 	return controller.adapter.CreateContainerFromPod(context.TODO(), pod)