diff --git a/pkg/apimachinery/apis/common/v0alpha1/secure_values.go b/pkg/apimachinery/apis/common/v0alpha1/secure_values.go index 3413753cee4..fd10d9c32a7 100644 --- a/pkg/apimachinery/apis/common/v0alpha1/secure_values.go +++ b/pkg/apimachinery/apis/common/v0alpha1/secure_values.go @@ -6,6 +6,9 @@ import ( "strconv" "gopkg.in/yaml.v3" + openapi "k8s.io/kube-openapi/pkg/common" + spec "k8s.io/kube-openapi/pkg/validation/spec" + ptr "k8s.io/utils/ptr" ) const redacted = "[REDACTED]" @@ -21,7 +24,8 @@ var ( _ yaml.Marshaler = (*RawSecureValue)(nil) ) -// Allow access to a secure value inside +// Reference a secure value from within an owner resource +// Only one property is valid at any time: OneOf(name,create,remove) // +k8s:openapi-gen=true type InlineSecureValue struct { // Create a secure value -- this is only used for POST/PUT @@ -30,6 +34,8 @@ type InlineSecureValue struct { Create RawSecureValue `json:"create,omitempty"` // Name in the secret service (reference) + // +k8s:validation:minLength=1 + // +k8s:validation:maxLength=253 Name string `json:"name,omitempty"` // Remove this value from the secure value map @@ -41,6 +47,50 @@ func (v InlineSecureValue) IsZero() bool { return v.Create.IsZero() && v.Name == "" && !v.Remove } +// OpenAPIDefinition returns the JSONSchema that manually ensures oneOf(create | name | remove) is set. +func (InlineSecureValue) OpenAPIDefinition() openapi.OpenAPIDefinition { + return openapi.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "Allow access to a secure value inside", + AdditionalProperties: &spec.SchemaOrBool{Allows: false}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name in the secret service (reference)", + Type: []string{"string"}, + MinLength: ptr.To[int64](1), + MaxLength: ptr.To[int64](253), + Format: ""}}, + "create": { + SchemaProps: spec.SchemaProps{ + Description: "Create a secure value -- this is only used for POST/PUT", + MinLength: ptr.To[int64](1), + MaxLength: ptr.To[int64](24576), + Type: []string{"string"}, + Format: ""}}, + "remove": { + SchemaProps: spec.SchemaProps{ + Description: "Remove this value from the secure value map Values owned by this resource will be deleted if necessary", + Type: []string{"boolean"}, + }}, + }, + OneOf: []spec.Schema{ + {SchemaProps: spec.SchemaProps{ + Required: []string{"name"}, + }}, + {SchemaProps: spec.SchemaProps{ + Required: []string{"create"}, + }}, + {SchemaProps: spec.SchemaProps{ + Required: []string{"remove"}, + }}, + }, + }, + }, + } +} + // Collection of secure values // +k8s:openapi-gen=true type InlineSecureValues = map[string]InlineSecureValue diff --git a/pkg/apimachinery/apis/common/v0alpha1/secure_values_test.go b/pkg/apimachinery/apis/common/v0alpha1/secure_values_test.go index 53f180753a9..83dd8ec6762 100644 --- a/pkg/apimachinery/apis/common/v0alpha1/secure_values_test.go +++ b/pkg/apimachinery/apis/common/v0alpha1/secure_values_test.go @@ -1,4 +1,4 @@ -package v0alpha1 +package v0alpha1_test import ( "bytes" @@ -8,40 +8,114 @@ import ( "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" + "k8s.io/kube-openapi/pkg/validation/strfmt" + "k8s.io/kube-openapi/pkg/validation/validate" + + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" ) func TestSecureValues(t *testing.T) { - expected := "[REDACTED]" + t.Run("redaction", func(t *testing.T) { + expected := "[REDACTED]" - rawValue := "a-password" - esv := NewSecretValue(rawValue) + rawValue := "a-password" + esv := common.NewSecretValue(rawValue) - // String must not return the exposed secure value. - require.Equal(t, expected, esv.String()) + // String must not return the exposed secure value. + require.Equal(t, expected, esv.String()) + require.Equal(t, expected, esv.GoString()) - // Format/GoString must not return the exposed secure value. - require.Equal(t, expected, fmt.Sprintf("%+#v", esv)) - require.Equal(t, expected, fmt.Sprintf("%v", esv)) - require.Equal(t, expected, fmt.Sprintf("%s", esv)) + // Format/GoString must not return the exposed secure value. + require.Equal(t, expected, fmt.Sprintf("%+#v", esv)) + require.Equal(t, expected, fmt.Sprintf("%v", esv)) + require.Equal(t, expected, fmt.Sprintf("%s", esv)) - buf := new(bytes.Buffer) - _, err := fmt.Fprintf(buf, "%#v", esv) - require.NoError(t, err) - require.Equal(t, expected, buf.String()) + buf := new(bytes.Buffer) + _, err := fmt.Fprintf(buf, "%#v", esv) + require.NoError(t, err) + require.Equal(t, expected, buf.String()) - // MarshalJSON must not return the exposed secure value. - bytes, err := json.Marshal(esv) - require.NoError(t, err) - require.Equal(t, `"`+expected+`"`, string(bytes)) + // MarshalJSON must not return the exposed secure value. + bytes, err := json.Marshal(esv) + require.NoError(t, err) + require.Equal(t, `"`+expected+`"`, string(bytes)) - // MarshalYAML must not return the exposed secure value. - bytes, err = yaml.Marshal(esv) - require.NoError(t, err) - require.Equal(t, "'"+expected+"'\n", string(bytes)) + // MarshalYAML must not return the exposed secure value. + bytes, err = yaml.Marshal(esv) + require.NoError(t, err) + require.Equal(t, "'"+expected+"'\n", string(bytes)) - // DangerouslyExposeAndConsumeValue returns the raw value. - require.Equal(t, rawValue, esv.DangerouslyExposeAndConsumeValue()) + // DangerouslyExposeAndConsumeValue returns the raw value. + require.Equal(t, rawValue, esv.DangerouslyExposeAndConsumeValue()) - // Further calls to DangerouslyExposeAndConsumeValue will panic. - require.Panics(t, func() { esv.DangerouslyExposeAndConsumeValue() }) + // Further calls to DangerouslyExposeAndConsumeValue will panic. + require.Panics(t, func() { esv.DangerouslyExposeAndConsumeValue() }) + }) + + t.Run("Inline", func(t *testing.T) { + t.Run("IsZero", func(t *testing.T) { + require.True(t, common.InlineSecureValue{}.IsZero()) + require.True(t, common.NewSecretValue("").IsZero()) + require.True(t, common.InlineSecureValue{Remove: false}.IsZero()) + + require.False(t, common.NewSecretValue("X").IsZero()) + require.False(t, common.InlineSecureValue{Name: "X"}.IsZero()) + require.False(t, common.InlineSecureValue{Remove: true}.IsZero()) + }) + + t.Run("Validate OneOf", func(t *testing.T) { + def := common.InlineSecureValue{}.OpenAPIDefinition() + // jj, _ := json.MarshalIndent(def.Schema, "", " ") + // fmt.Printf("%s", string(jj)) + // t.FailNow() + + validator := validate.NewSchemaValidator(&def.Schema, nil, "", strfmt.Default) + + tests := []struct { + name string + input map[string]any + valid bool + err string + }{ + { + name: "with name", + input: map[string]any{"name": "x"}, + valid: true, + }, + { + name: "with create", + input: map[string]any{"create": "x"}, + valid: true, + }, + { + name: "with remove", + input: map[string]any{"remove": true}, + valid: true, + }, + { + name: "empty", + input: map[string]any{}, + valid: false, + err: "must validate one and only one", + }, + { + name: "with unknown property", + input: map[string]any{"unknown": "property"}, + valid: false, + err: "unknown", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := validator.Validate(tt.input) + require.Equal(t, tt.valid, result.IsValid()) + if tt.err == "" { + require.NoError(t, result.AsError()) + } else { + require.ErrorContains(t, result.AsError(), tt.err) + } + }) + } + }) + }) } diff --git a/pkg/apimachinery/apis/common/v0alpha1/zz_generated.openapi.go b/pkg/apimachinery/apis/common/v0alpha1/zz_generated.openapi.go index 2f92840b5e6..34424e88e3d 100644 --- a/pkg/apimachinery/apis/common/v0alpha1/zz_generated.openapi.go +++ b/pkg/apimachinery/apis/common/v0alpha1/zz_generated.openapi.go @@ -11,12 +11,11 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" common "k8s.io/kube-openapi/pkg/common" spec "k8s.io/kube-openapi/pkg/validation/spec" - ptr "k8s.io/utils/ptr" ) func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ - "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.InlineSecureValue": schema_apimachinery_apis_common_v0alpha1_InlineSecureValue(ref), + "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.InlineSecureValue": InlineSecureValue{}.OpenAPIDefinition(), "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.ObjectReference": schema_apimachinery_apis_common_v0alpha1_ObjectReference(ref), "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.Scope": schema_apimachinery_apis_common_v0alpha1_Scope(ref), "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.ScopeFilter": schema_apimachinery_apis_common_v0alpha1_ScopeFilter(ref), @@ -78,42 +77,6 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA } } -func schema_apimachinery_apis_common_v0alpha1_InlineSecureValue(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Description: "Allow access to a secure value inside", - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "create": { - SchemaProps: spec.SchemaProps{ - Description: "Create a secure value -- this is only used for POST/PUT", - MinLength: ptr.To[int64](1), - MaxLength: ptr.To[int64](24576), - Type: []string{"string"}, - Format: "", - }, - }, - "name": { - SchemaProps: spec.SchemaProps{ - Description: "Name in the secret service (reference)", - Type: []string{"string"}, - Format: "", - }, - }, - "remove": { - SchemaProps: spec.SchemaProps{ - Description: "Remove this value from the secure value map Values owned by this resource will be deleted if necessary", - Type: []string{"boolean"}, - Format: "", - }, - }, - }, - }, - }, - } -} - func schema_apimachinery_apis_common_v0alpha1_ObjectReference(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apimachinery/utils/meta_test.go b/pkg/apimachinery/utils/meta_test.go index 54db0fbe976..1e79f267430 100644 --- a/pkg/apimachinery/utils/meta_test.go +++ b/pkg/apimachinery/utils/meta_test.go @@ -157,7 +157,10 @@ func TestMetaAccessor(t *testing.T) { } t.Run("fails for non resource objects", func(t *testing.T) { - _, err := utils.MetaAccessor("hello") + _, err := utils.MetaAccessor(nil) + require.Error(t, err) + + _, err = utils.MetaAccessor("hello") require.Error(t, err) _, err = utils.MetaAccessor(unstructured.Unstructured{}) @@ -174,6 +177,45 @@ func TestMetaAccessor(t *testing.T) { require.NoError(t, err) // Must be a pointer }) + t.Run("get and set properties", func(t *testing.T) { + obj, err := utils.MetaAccessor(&unstructured.Unstructured{ + Object: map[string]any{}, + }) + require.NoError(t, err) + rv := obj.GetResourceVersion() + require.Equal(t, "", rv) + + _, ok := obj.GetRuntimeObject() + require.True(t, ok) + anno := obj.GetAnnotations() + require.Nil(t, anno) + + rvInt, err := obj.GetResourceVersionInt64() + require.NoError(t, err) + require.Equal(t, int64(0), rvInt) + + obj.SetResourceVersion("not a number") + rv = obj.GetResourceVersion() + require.Equal(t, "not a number", rv) + rvInt, err = obj.GetResourceVersionInt64() + require.Error(t, err) + require.Equal(t, int64(0), rvInt) + + obj.SetUpdatedBy("updatedBy") + require.Equal(t, "updatedBy", obj.GetUpdatedBy()) + anno = obj.GetAnnotations() + require.Len(t, anno, 1) // One key + obj.SetAnnotation(utils.AnnoKeyUpdatedBy, "") + anno = obj.GetAnnotations() + require.Empty(t, anno) // removed the key + + obj.SetCreatedBy("createdBy") + require.Equal(t, "createdBy", obj.GetCreatedBy()) + + obj.SetFolder("folder") + require.Equal(t, "folder", obj.GetFolder()) + }) + t.Run("get and set grafana labels (unstructured)", func(t *testing.T) { res := &unstructured.Unstructured{ Object: map[string]any{}, @@ -593,6 +635,32 @@ func TestMetaAccessor(t *testing.T) { } }) + t.Run("manage secure values", func(t *testing.T) { + raw := &unstructured.Unstructured{ + Object: map[string]any{}, + } + obj, _ := utils.MetaAccessor(raw) + sv, err := obj.GetSecureValues() + require.NoError(t, err) + require.Nil(t, sv) + + err = obj.SetSecureValues(common.InlineSecureValues{ + "A": common.InlineSecureValue{Name: "NameForA"}, + }) + require.NoError(t, err) + require.NotNil(t, raw.Object["secure"]) + sv, err = obj.GetSecureValues() + require.NoError(t, err) + require.Equal(t, "NameForA", sv["A"].Name) + + // Manually set secure to an invalid property: + raw.Object["secure"] = t + sv, err = obj.GetSecureValues() + require.Error(t, err) + require.Nil(t, sv) + delete(raw.Object, "secure") + }) + t.Run("SourceProperties", func(t *testing.T) { tests := []struct { name string