Skip to content

Commit

Permalink
fix: Cover additional Secretref validation scenarios (#1535)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TeodorSAP
TeodorSAP authored Oct 25, 2024
1 parent 49f42ed commit a3d4f8b
Show file tree
Hide file tree
Showing 35 changed files with 865 additions and 128 deletions.
4 changes: 2 additions & 2 deletions apis/telemetry/v1alpha1/logpipeline_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ type Output struct {
OTLP *OTLPOutput `json:"otlp,omitempty"`
}

func (i *Input) IsDefined() bool {
func (i *Input) IsValid() bool {
return i != nil
}

Expand All @@ -157,7 +157,7 @@ func (o *Output) IsCustomDefined() bool {
}

func (o *Output) IsHTTPDefined() bool {
return o.HTTP != nil && o.HTTP.Host.IsDefined()
return o.HTTP != nil && o.HTTP.Host.IsValid()
}

func (o *Output) IsOTLPDefined() bool {
Expand Down
2 changes: 1 addition & 1 deletion apis/telemetry/v1alpha1/logpipeline_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func validateCustomFilter(content string) error {

func (lp *LogPipeline) validateInput() error {
input := lp.Spec.Input
if !input.IsDefined() {
if !input.IsValid() {
return nil
}

Expand Down
10 changes: 5 additions & 5 deletions apis/telemetry/v1alpha1/secret_refs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ func (lp *LogPipeline) GetSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

for _, v := range lp.Spec.Variables {
if v.ValueFrom.IsSecretKeyRef() {
if v.ValueFrom.SecretKeyRef != nil {
refs = append(refs, *v.ValueFrom.SecretKeyRef)
}
}
Expand All @@ -20,7 +20,7 @@ func (lp *LogPipeline) GetEnvSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

output := lp.Spec.Output
if output.IsHTTPDefined() {
if output.HTTP != nil {
refs = appendIfSecretRef(refs, output.HTTP.Host)
refs = appendIfSecretRef(refs, output.HTTP.User)
refs = appendIfSecretRef(refs, output.HTTP.Password)
Expand All @@ -33,7 +33,7 @@ func (lp *LogPipeline) GetTLSSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

output := lp.Spec.Output
if output.IsHTTPDefined() {
if output.HTTP != nil {
tlsConfig := output.HTTP.TLSConfig
if tlsConfig.CA != nil {
refs = appendIfSecretRef(refs, *tlsConfig.CA)
Expand Down Expand Up @@ -64,7 +64,7 @@ func getRefsInOTLPOutput(otlpOut *OTLPOutput) []SecretKeyRef {

refs = appendIfSecretRef(refs, otlpOut.Endpoint)

if otlpOut.Authentication != nil && otlpOut.Authentication.Basic.IsDefined() {
if otlpOut.Authentication != nil && otlpOut.Authentication.Basic != nil {
refs = appendIfSecretRef(refs, otlpOut.Authentication.Basic.User)
refs = appendIfSecretRef(refs, otlpOut.Authentication.Basic.Password)
}
Expand All @@ -91,7 +91,7 @@ func getRefsInOTLPOutput(otlpOut *OTLPOutput) []SecretKeyRef {
}

func appendIfSecretRef(secretKeyRefs []SecretKeyRef, valueType ValueType) []SecretKeyRef {
if valueType.Value == "" && valueType.ValueFrom != nil && valueType.ValueFrom.IsSecretKeyRef() {
if valueType.Value == "" && valueType.ValueFrom != nil && valueType.ValueFrom.SecretKeyRef != nil {
secretKeyRefs = append(secretKeyRefs, *valueType.ValueFrom.SecretKeyRef)
}

Expand Down
132 changes: 131 additions & 1 deletion apis/telemetry/v1alpha1/secret_refs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,46 @@ func TestLogPipeline_GetSecretRefs(t *testing.T) {
{Name: "creds", Namespace: "default", Key: "password"},
},
},
{
name: "http output secret refs (with missing fields)",
given: LogPipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "cls",
},
Spec: LogPipelineSpec{
Output: Output{
HTTP: &HTTPOutput{
Host: ValueType{
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "creds", Namespace: "default",
},
},
},
User: ValueType{
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "creds", Key: "user",
},
},
},
Password: ValueType{
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Namespace: "default", Key: "password",
},
},
},
},
},
},
},
expected: []SecretKeyRef{
{Name: "creds", Namespace: "default"},
{Name: "creds", Key: "user"},
{Namespace: "default", Key: "password"},
},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -162,6 +202,51 @@ func TestTracePipeline_GetSecretRefs(t *testing.T) {
{Name: "secret-3", Namespace: "default", Key: "myheader"},
},
},
{
name: "basic auth and header (with missing fields)",
pipelineName: "test-pipeline",
given: &OTLPOutput{
Authentication: &AuthenticationOptions{
Basic: &BasicAuthOptions{
User: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "secret-1",
Key: "user",
}},
},
Password: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Namespace: "default",
Key: "password",
}},
},
},
},
Headers: []Header{
{
Name: "header-1",
ValueType: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "secret-3",
Namespace: "default",
}},
},
},
},
},

expected: []SecretKeyRef{
{Name: "secret-1", Key: "user"},
{Namespace: "default", Key: "password"},
{Name: "secret-3", Namespace: "default"},
},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -199,7 +284,7 @@ func TestMetricPipeline_GetSecretRefs(t *testing.T) {
},
},
{
name: "basic auth and",
name: "basic auth and header",
pipelineName: "test-pipeline",
given: &OTLPOutput{
Authentication: &AuthenticationOptions{
Expand Down Expand Up @@ -246,6 +331,51 @@ func TestMetricPipeline_GetSecretRefs(t *testing.T) {
{Name: "secret-3", Namespace: "default", Key: "myheader"},
},
},
{
name: "basic auth and header (with missing fields)",
pipelineName: "test-pipeline",
given: &OTLPOutput{
Authentication: &AuthenticationOptions{
Basic: &BasicAuthOptions{
User: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Namespace: "default",
Key: "user",
}},
},
Password: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "secret-2",
Key: "password",
}},
},
},
},
Headers: []Header{
{
Name: "header-1",
ValueType: ValueType{
Value: "",
ValueFrom: &ValueFromSource{
SecretKeyRef: &SecretKeyRef{
Name: "secret-3",
Namespace: "default",
}},
},
},
},
},

expected: []SecretKeyRef{
{Namespace: "default", Key: "user"},
{Name: "secret-2", Key: "password"},
{Name: "secret-3", Namespace: "default"},
},
},
}

for _, test := range tests {
Expand Down
19 changes: 9 additions & 10 deletions apis/telemetry/v1alpha1/shared_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type ValueType struct {
ValueFrom *ValueFromSource `json:"valueFrom,omitempty"`
}

func (v *ValueType) IsDefined() bool {
func (v *ValueType) IsValid() bool {
if v == nil {
return false
}
Expand All @@ -20,24 +20,27 @@ func (v *ValueType) IsDefined() bool {
return true
}

return v.ValueFrom != nil && v.ValueFrom.IsSecretKeyRef()
return v.ValueFrom != nil &&
v.ValueFrom.SecretKeyRef != nil &&
v.ValueFrom.SecretKeyRef.Name != "" &&
v.ValueFrom.SecretKeyRef.Key != "" &&
v.ValueFrom.SecretKeyRef.Namespace != ""
}

type ValueFromSource struct {
// Refers to the value of a specific key in a Secret. You must provide `name` and `namespace` of the Secret, as well as the name of the `key`.
SecretKeyRef *SecretKeyRef `json:"secretKeyRef,omitempty"`
}

func (v *ValueFromSource) IsSecretKeyRef() bool {
return v.SecretKeyRef != nil && v.SecretKeyRef.Name != "" && v.SecretKeyRef.Key != ""
}

type SecretKeyRef struct {
// The name of the Secret containing the referenced value
// +kubebuilder:validation:Required
Name string `json:"name,omitempty"`
// The name of the Namespace containing the Secret with the referenced value.
// +kubebuilder:validation:Required
Namespace string `json:"namespace,omitempty"`
// The name of the attribute of the Secret holding the referenced value.
// +kubebuilder:validation:Required
Key string `json:"key,omitempty"`
}

Expand Down Expand Up @@ -108,10 +111,6 @@ type BasicAuthOptions struct {
Password ValueType `json:"password"`
}

func (b *BasicAuthOptions) IsDefined() bool {
return b.User.IsDefined() && b.Password.IsDefined()
}

// OTLPInput defines the collection of push-based metrics that use the OpenTelemetry protocol.
type OTLPInput struct {
// If disabled, push-based OTLP signals are not collected. The default is `false`.
Expand Down
4 changes: 2 additions & 2 deletions apis/telemetry/v1beta1/logpipeline_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func init() {
SchemeBuilder.Register(&LogPipeline{}, &LogPipelineList{})
}

func (i *LogPipelineInput) IsDefined() bool {
func (i *LogPipelineInput) IsValid() bool {
return i != nil
}

Expand All @@ -207,7 +207,7 @@ func (o *LogPipelineOutput) IsCustomDefined() bool {
}

func (o *LogPipelineOutput) IsHTTPDefined() bool {
return o.HTTP != nil && o.HTTP.Host.IsDefined()
return o.HTTP != nil && o.HTTP.Host.IsValid()
}

func (o *LogPipelineOutput) IsAnyDefined() bool {
Expand Down
2 changes: 1 addition & 1 deletion apis/telemetry/v1beta1/logpipeline_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func validateCustomFilter(content string) error {

func (lp *LogPipeline) validateInput() error {
input := lp.Spec.Input
if !input.IsDefined() {
if !input.IsValid() {
return nil
}

Expand Down
10 changes: 5 additions & 5 deletions apis/telemetry/v1beta1/secret_refs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ func (lp *LogPipeline) GetSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

for _, v := range lp.Spec.Variables {
if v.ValueFrom.IsSecretKeyRef() {
if v.ValueFrom.SecretKeyRef != nil {
refs = append(refs, *v.ValueFrom.SecretKeyRef)
}
}
Expand All @@ -20,7 +20,7 @@ func (lp *LogPipeline) GetEnvSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

output := lp.Spec.Output
if output.IsHTTPDefined() {
if output.HTTP != nil {
refs = appendIfSecretRef(refs, output.HTTP.Host)
refs = appendIfSecretRef(refs, output.HTTP.User)
refs = appendIfSecretRef(refs, output.HTTP.Password)
Expand All @@ -33,7 +33,7 @@ func (lp *LogPipeline) GetTLSSecretRefs() []SecretKeyRef {
var refs []SecretKeyRef

output := lp.Spec.Output
if output.IsHTTPDefined() {
if output.HTTP != nil {
tlsConfig := output.HTTP.TLSConfig
if tlsConfig.CA != nil {
refs = appendIfSecretRef(refs, *tlsConfig.CA)
Expand Down Expand Up @@ -64,7 +64,7 @@ func getRefsInOTLPOutput(out *OTLPOutput) []SecretKeyRef {

refs = appendIfSecretRef(refs, out.Endpoint)

if out.Authentication != nil && out.Authentication.Basic.IsDefined() {
if out.Authentication != nil && out.Authentication.Basic != nil {
refs = appendIfSecretRef(refs, out.Authentication.Basic.User)
refs = appendIfSecretRef(refs, out.Authentication.Basic.Password)
}
Expand All @@ -91,7 +91,7 @@ func getRefsInOTLPOutput(out *OTLPOutput) []SecretKeyRef {
}

func appendIfSecretRef(secretKeyRefs []SecretKeyRef, valueType ValueType) []SecretKeyRef {
if valueType.Value == "" && valueType.ValueFrom != nil && valueType.ValueFrom.IsSecretKeyRef() {
if valueType.Value == "" && valueType.ValueFrom != nil && valueType.ValueFrom.SecretKeyRef != nil {
secretKeyRefs = append(secretKeyRefs, *valueType.ValueFrom.SecretKeyRef)
}

Expand Down
Loading

0 comments on commit a3d4f8b

Please sign in to comment.