Secrets: Validate name/namespace with standard K8s validator (#109868)
* Secrets: Validate name/namespace with standard K8s validator * Secrets: Simplify error message for mismatched owner inline secure values
This commit is contained in:
@@ -11,7 +11,7 @@ import (
|
||||
)
|
||||
|
||||
// The maximum size of a secure value in bytes when written as raw input.
|
||||
const SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES = 24576 // 24 KiB
|
||||
const SecureValueRawInputMaxSizeBytes = 24576 // 24 KiB
|
||||
|
||||
type DecryptSecureValue struct {
|
||||
Keeper *string
|
||||
|
||||
@@ -118,10 +118,7 @@ func (s *LocalInlineSecureValueService) isSecureValueOwnedByResource(ctx context
|
||||
return true, nil // The secure value is owned by the same owner reference, pass!
|
||||
}
|
||||
|
||||
return false, fmt.Errorf(
|
||||
"secure value %s is not owned by %s/%s/%s/%s but by %s/%s/%s",
|
||||
name, owner.APIGroup, owner.APIVersion, owner.Kind, owner.Name, actualOwner.APIVersion, actualOwner.Kind, actualOwner.Name,
|
||||
)
|
||||
return false, fmt.Errorf("secure value %s is not owned by %s/%s/%s/%s", name, owner.APIGroup, owner.APIVersion, owner.Kind, owner.Name)
|
||||
}
|
||||
|
||||
// not owned
|
||||
|
||||
@@ -3,6 +3,7 @@ package validator
|
||||
import (
|
||||
"strings"
|
||||
|
||||
"k8s.io/apimachinery/pkg/util/validation"
|
||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||
"k8s.io/apiserver/pkg/admission"
|
||||
|
||||
@@ -19,12 +20,26 @@ func ProvideKeeperValidator() contracts.KeeperValidator {
|
||||
}
|
||||
|
||||
func (v *keeperValidator) Validate(keeper *secretv1beta1.Keeper, oldKeeper *secretv1beta1.Keeper, operation admission.Operation) field.ErrorList {
|
||||
// Only validate Create and Update for now.
|
||||
if operation != admission.Create && operation != admission.Update {
|
||||
return nil
|
||||
errs := make(field.ErrorList, 0)
|
||||
|
||||
// General validations.
|
||||
if err := validation.IsDNS1123Subdomain(keeper.Name); len(err) > 0 {
|
||||
errs = append(
|
||||
errs,
|
||||
field.Invalid(field.NewPath("metadata", "name"), keeper.Name, strings.Join(err, ",")),
|
||||
)
|
||||
}
|
||||
if err := validation.IsDNS1123Subdomain(keeper.Namespace); len(err) > 0 {
|
||||
errs = append(
|
||||
errs,
|
||||
field.Invalid(field.NewPath("metadata", "namespace"), keeper.Name, strings.Join(err, ",")),
|
||||
)
|
||||
}
|
||||
|
||||
errs := make(field.ErrorList, 0)
|
||||
// Only validate Create and Update for now.
|
||||
if operation != admission.Create && operation != admission.Update {
|
||||
return errs
|
||||
}
|
||||
|
||||
if keeper.Spec.Description == "" {
|
||||
errs = append(errs, field.Required(field.NewPath("spec", "description"), "a `description` is required"))
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
package validator
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apiserver/pkg/admission"
|
||||
"k8s.io/utils/ptr"
|
||||
|
||||
@@ -11,11 +13,13 @@ import (
|
||||
)
|
||||
|
||||
func TestValidateKeeper(t *testing.T) {
|
||||
objectMeta := metav1.ObjectMeta{Name: "test", Namespace: "test"}
|
||||
validator := ProvideKeeperValidator()
|
||||
|
||||
t.Run("when creating a new keeper", func(t *testing.T) {
|
||||
t.Run("the `description` must be present", func(t *testing.T) {
|
||||
keeper := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Aws: &secretv1beta1.KeeperAWSConfig{
|
||||
AccessKeyID: secretv1beta1.KeeperCredentialValue{ValueFromEnv: "some-value"},
|
||||
@@ -33,6 +37,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("only one `keeper` must be present", func(t *testing.T) {
|
||||
keeper := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "short description",
|
||||
Aws: &secretv1beta1.KeeperAWSConfig{},
|
||||
@@ -49,6 +54,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("at least one `keeper` must be present", func(t *testing.T) {
|
||||
keeper := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
},
|
||||
@@ -61,6 +67,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("aws keeper validation", func(t *testing.T) {
|
||||
validKeeperAWS := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
Aws: &secretv1beta1.KeeperAWSConfig{
|
||||
@@ -126,6 +133,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("azure keeper validation", func(t *testing.T) {
|
||||
validKeeperAzure := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
Azure: &secretv1beta1.KeeperAzureConfig{
|
||||
@@ -193,6 +201,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("gcp keeper validation", func(t *testing.T) {
|
||||
validKeeperGCP := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
Gcp: &secretv1beta1.KeeperGCPConfig{
|
||||
@@ -223,6 +232,7 @@ func TestValidateKeeper(t *testing.T) {
|
||||
|
||||
t.Run("hashicorp keeper validation", func(t *testing.T) {
|
||||
validKeeperHashiCorp := &secretv1beta1.Keeper{
|
||||
ObjectMeta: objectMeta,
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
HashiCorpVault: &secretv1beta1.KeeperHashiCorpConfig{
|
||||
@@ -267,4 +277,68 @@ func TestValidateKeeper(t *testing.T) {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("invalid name", func(t *testing.T) {
|
||||
keeper := &secretv1beta1.Keeper{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: objectMeta.Namespace,
|
||||
},
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
HashiCorpVault: &secretv1beta1.KeeperHashiCorpConfig{
|
||||
Address: "http://address",
|
||||
Token: secretv1beta1.KeeperCredentialValue{
|
||||
ValueFromConfig: "config.path.value",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
keeper.Name = ""
|
||||
errs := validator.Validate(keeper, nil, admission.Delete)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
|
||||
keeper.Name = "invalid/name-"
|
||||
errs = validator.Validate(keeper, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
|
||||
keeper.Name = strings.Repeat("a", 253+1)
|
||||
errs = validator.Validate(keeper, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
})
|
||||
|
||||
t.Run("invalid namespace", func(t *testing.T) {
|
||||
keeper := &secretv1beta1.Keeper{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: objectMeta.Name,
|
||||
},
|
||||
Spec: secretv1beta1.KeeperSpec{
|
||||
Description: "description",
|
||||
HashiCorpVault: &secretv1beta1.KeeperHashiCorpConfig{
|
||||
Address: "http://address",
|
||||
Token: secretv1beta1.KeeperCredentialValue{
|
||||
ValueFromConfig: "config.path.value",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
keeper.Namespace = ""
|
||||
errs := validator.Validate(keeper, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
|
||||
keeper.Namespace = "invalid/namespace-"
|
||||
errs = validator.Validate(keeper, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
|
||||
keeper.Namespace = strings.Repeat("a", 253+1)
|
||||
errs = validator.Validate(keeper, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -38,17 +38,27 @@ func (v *secureValueValidator) Validate(sv, oldSv *secretv1beta1.SecureValue, op
|
||||
}
|
||||
|
||||
// General validations.
|
||||
if sv.Name == "" {
|
||||
errs = append(errs, field.Required(field.NewPath("metadata", "name"), "a `name` is required"))
|
||||
}
|
||||
if sv.Namespace == "" {
|
||||
errs = append(errs, field.Required(field.NewPath("metadata", "namespace"), "a `namespace` is required"))
|
||||
}
|
||||
|
||||
if sv.Spec.Value != nil && len(*sv.Spec.Value) > contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES {
|
||||
if err := validation.IsDNS1123Subdomain(sv.Name); len(err) > 0 {
|
||||
errs = append(
|
||||
errs,
|
||||
field.TooLong(field.NewPath("spec", "value"), len(*sv.Spec.Value), contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES),
|
||||
field.Invalid(field.NewPath("metadata", "name"), sv.Name, strings.Join(err, ",")),
|
||||
)
|
||||
|
||||
return errs
|
||||
}
|
||||
if err := validation.IsDNS1123Subdomain(sv.Namespace); len(err) > 0 {
|
||||
errs = append(
|
||||
errs,
|
||||
field.Invalid(field.NewPath("metadata", "namespace"), sv.Name, strings.Join(err, ",")),
|
||||
)
|
||||
|
||||
return errs
|
||||
}
|
||||
|
||||
if sv.Spec.Value != nil && len(*sv.Spec.Value) > contracts.SecureValueRawInputMaxSizeBytes {
|
||||
errs = append(
|
||||
errs,
|
||||
field.TooLong(field.NewPath("spec", "value"), len(*sv.Spec.Value), contracts.SecureValueRawInputMaxSizeBytes),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -69,7 +69,7 @@ func TestValidateSecureValue(t *testing.T) {
|
||||
|
||||
t.Run("`value` cannot exceed 24576 bytes", func(t *testing.T) {
|
||||
sv := validSecureValue.DeepCopy()
|
||||
sv.Spec.Value = ptr.To(secretv1beta1.NewExposedSecureValue(strings.Repeat("a", contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES+1)))
|
||||
sv.Spec.Value = ptr.To(secretv1beta1.NewExposedSecureValue(strings.Repeat("a", contracts.SecureValueRawInputMaxSizeBytes+1)))
|
||||
sv.Spec.Ref = nil
|
||||
|
||||
errs := validator.Validate(sv, nil, admission.Create)
|
||||
@@ -288,4 +288,58 @@ func TestValidateSecureValue(t *testing.T) {
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "spec.decrypters", errs[0].Field)
|
||||
})
|
||||
|
||||
t.Run("invalid name", func(t *testing.T) {
|
||||
sv := &secretv1beta1.SecureValue{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: objectMeta.Namespace,
|
||||
},
|
||||
Spec: secretv1beta1.SecureValueSpec{
|
||||
Description: "description",
|
||||
Ref: ptr.To("ref"),
|
||||
},
|
||||
}
|
||||
|
||||
sv.Name = ""
|
||||
errs := validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
|
||||
sv.Name = "invalid/name-"
|
||||
errs = validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
|
||||
sv.Name = strings.Repeat("a", 253+1)
|
||||
errs = validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.name", errs[0].Field)
|
||||
})
|
||||
|
||||
t.Run("invalid namespace", func(t *testing.T) {
|
||||
sv := &secretv1beta1.SecureValue{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: objectMeta.Name,
|
||||
},
|
||||
Spec: secretv1beta1.SecureValueSpec{
|
||||
Description: "description",
|
||||
Ref: ptr.To("ref"),
|
||||
},
|
||||
}
|
||||
|
||||
sv.Namespace = ""
|
||||
errs := validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
|
||||
sv.Namespace = "invalid/namespace-"
|
||||
errs = validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
|
||||
sv.Namespace = strings.Repeat("a", 253+1)
|
||||
errs = validator.Validate(sv, nil, admission.Create)
|
||||
require.Len(t, errs, 1)
|
||||
require.Equal(t, "metadata.namespace", errs[0].Field)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user