Merge pull request #232 from vshn/forbidden_fields_validate

Validate against forbidden fields

pkg/controller/webhooks/postgresql.go

@@ -2,6 +2,7 @@ package webhooks
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	vshnv1 "github.com/vshn/appcat/v4/apis/vshn/v1"
@@ -34,6 +35,24 @@ var (
 
 var _ webhook.CustomValidator = &PostgreSQLWebhookHandler{}
 
+var blocklist = map[string]string{
+	"listen_addresses":      "",
+	"port":                  "",
+	"cluster_name":          "",
+	"hot_standby":           "",
+	"fsync":                 "",
+	"full_page_writes":      "",
+	"log_destination":       "",
+	"logging_collector":     "",
+	"max_replication_slots": "",
+	"max_wal_senders":       "",
+	"wal_keep_segments":     "",
+	"wal_level":             "",
+	"wal_log_hints":         "",
+	"archive_mode":          "",
+	"archive_command":       "",
+}
+
 // PostgreSQLWebhookHandler handles all quota webhooks concerning postgresql by vshn.
 type PostgreSQLWebhookHandler struct {
 	DefaultWebhookHandler
@@ -105,6 +124,11 @@ func (p *PostgreSQLWebhookHandler) ValidateCreate(ctx context.Context, obj runti
 		})
 	}
 
+	errList := validatePgConf(pg)
+	if errList != nil {
+		allErrs = append(allErrs, errList...)
+	}
+
 	if len(allErrs) != 0 {
 		return nil, apierrors.NewInvalid(
 			pgGK,
@@ -167,6 +191,11 @@ func (p *PostgreSQLWebhookHandler) ValidateUpdate(ctx context.Context, oldObj, n
 		})
 	}
 
+	errList := validatePgConf(pg)
+	if errList != nil {
+		allErrs = append(allErrs, errList...)
+	}
+
 	// We aggregate and return all errors at the same time.
 	// So the user is aware of all broken parameters.
 	// But at the same time, if any of these fail we cannot do proper quota checks anymore.
@@ -287,3 +316,34 @@ func validateVacuumRepack(vacuum, repack bool) error {
 	}
 	return nil
 }
+
+func validatePgConf(pg *vshnv1.VSHNPostgreSQL) (fErros field.ErrorList) {
+
+	pgConfBytes := pg.Spec.Parameters.Service.PostgreSQLSettings
+
+	pgConf := map[string]string{}
+	if pgConfBytes.Raw != nil {
+		err := json.Unmarshal(pgConfBytes.Raw, &pgConf)
+		if err != nil {
+			fErros = append(fErros, &field.Error{
+				Field:    "spec.parameters.service.postgresqlSettings",
+				Detail:   fmt.Sprintf("Error parsing pgConf: %s", err.Error()),
+				Type:     field.ErrorTypeInvalid,
+				BadValue: pgConfBytes,
+			})
+			return fErros
+		}
+	}
+
+	for key := range pgConf {
+		if _, ok := blocklist[key]; ok {
+			fErros = append(fErros, &field.Error{
+				Field:    fmt.Sprintf("spec.parameters.service.postgresqlSettings[%s]", key),
+				Type:     field.ErrorTypeForbidden,
+				BadValue: key,
+				Detail:   "https://stackgres.io/doc/latest/api/responses/error/#postgres-blocklist",
+			})
+		}
+	}
+	return fErros
+}

pkg/controller/webhooks/postgresql_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/vshn/appcat/v4/pkg/common/utils"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
@@ -231,6 +232,103 @@ func TestPostgreSQLWebhookHandler_ValidateCreate(t *testing.T) {
 	pgInvalid.Spec.Parameters.Service.ServiceLevel = "guaranteed"
 	_, err = handler.ValidateCreate(ctx, pgInvalid)
 	assert.Error(t, err)
+
+	// check pgSettings
+	pgValid := pgOrig.DeepCopy()
+	pgValid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"foo": "bar"}`),
+	}
+	_, err = handler.ValidateCreate(ctx, pgValid)
+	assert.NoError(t, err)
+
+	// check pgSettings
+	pgInvalid = pgOrig.DeepCopy()
+	pgInvalid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"fsync": "bar"}`),
+	}
+	_, err = handler.ValidateCreate(ctx, pgInvalid)
+	assert.Error(t, err)
+
+	// check pgSettings
+	pgInvalid = pgOrig.DeepCopy()
+	pgInvalid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"fsync": "bar", "wal_level": "foo", "max_connections": "bar"}`),
+	}
+
+	_, err = handler.ValidateCreate(ctx, pgInvalid)
+	assert.Error(t, err)
+}
+
+func TestPostgreSQLWebhookHandler_ValidateUpdate(t *testing.T) {
+	ctx := context.TODO()
+	claimNS := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "claimns",
+			Labels: map[string]string{
+				utils.OrgLabelName: "myorg",
+			},
+		},
+	}
+	fclient := fake.NewClientBuilder().
+		WithScheme(pkg.SetupScheme()).
+		WithObjects(claimNS).
+		Build()
+
+	viper.Set("PLANS_NAMESPACE", "testns")
+	viper.AutomaticEnv()
+
+	handler := PostgreSQLWebhookHandler{
+		DefaultWebhookHandler: DefaultWebhookHandler{
+			client:    fclient,
+			log:       logr.Discard(),
+			withQuota: false,
+			obj:       &vshnv1.VSHNPostgreSQL{},
+			name:      "postgresql",
+		},
+	}
+	pgOrig := &vshnv1.VSHNPostgreSQL{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "myinstance",
+			Namespace: "claimns",
+		},
+		Spec: vshnv1.VSHNPostgreSQLSpec{
+			Parameters: vshnv1.VSHNPostgreSQLParameters{
+				Size: vshnv1.VSHNSizeSpec{
+					CPU:    "500m",
+					Memory: "1Gi",
+				},
+				Instances: 1,
+				Service: vshnv1.VSHNPostgreSQLServiceSpec{
+					RepackEnabled: true,
+				},
+			},
+		},
+	}
+
+	// check pgSettings with single good setting
+	pgValid := pgOrig.DeepCopy()
+	pgValid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"foo": "bar"}`),
+	}
+	_, err := handler.ValidateUpdate(ctx, pgOrig, pgValid)
+	assert.NoError(t, err)
+
+	// check pgSettings with single bad setting
+	pgInvalid := pgOrig.DeepCopy()
+	pgInvalid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"fsync": "bar"}`),
+	}
+	_, err = handler.ValidateUpdate(ctx, pgOrig, pgInvalid)
+	assert.Error(t, err)
+
+	// check pgSettings, startiong with valid settings
+	pgInvalid = pgOrig.DeepCopy()
+	pgInvalid.Spec.Parameters.Service.PostgreSQLSettings = runtime.RawExtension{
+		Raw: []byte(`{"foo": "bar", "fsync": "bar", "wal_level": "foo", "max_connections": "bar"}`),
+	}
+
+	_, err = handler.ValidateUpdate(ctx, pgOrig, pgInvalid)
+	assert.Error(t, err)
 }
 
 func TestPostgreSQLWebhookHandler_ValidateDelete(t *testing.T) {