Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(regression): Support input path globbing in Go re-write #23

Merged
merged 9 commits into from
Jun 4, 2024
46 changes: 31 additions & 15 deletions action/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func (a *Action) Apply() error {
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were logging the wrong resource type here, just cosmetic.


if a.sourcePath != "" {
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindSource)), zap.String("file", a.destinationPath))
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindSource)), zap.String("file", a.sourcePath))
err := a.apply(a.sourcePath)
if err != nil {
return fmt.Errorf("sources: %w", err)
Expand All @@ -251,7 +251,7 @@ func (a *Action) Apply() error {
}

if a.processorPath != "" {
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindProcessor)), zap.String("file", a.destinationPath))
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindProcessor)), zap.String("file", a.processorPath))
err := a.apply(a.processorPath)
if err != nil {
return fmt.Errorf("processors: %w", err)
Expand All @@ -261,7 +261,7 @@ func (a *Action) Apply() error {
}

if a.configurationPath != "" {
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindConfiguration)), zap.String("file", a.destinationPath))
a.Logger.Info("Applying resources", zap.String("Kind", string(model.KindConfiguration)), zap.String("file", a.configurationPath))
err := a.apply(a.configurationPath)
if err != nil {
return fmt.Errorf("configuration: %w", err)
Expand Down Expand Up @@ -491,25 +491,41 @@ func (a *Action) WriteBack() error {

// decodeAnyResourceFile takes a file path and decodes it into a slice of
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same logic, just inside a for loop iterating over matches found by the globbing. If it is a single file without glob chars, it will just iterate once on that file.

// model.AnyResource. If the file is empty, it will return an error.
// This function supports globbing, but does not gaurantee ordering. This
// function should not be passed multiple files with differing resource
// types such as Destinations and Configurations.
func decodeAnyResourceFile(path string) ([]*model.AnyResource, error) {
f, err := os.Open(path) // #nosec G304 user defined filepath
// Glob will return nil matches if there are IO errors. Glob only returns
// an error if an invalid pattern is given.
matches, err := filepath.Glob(path) // #nosec G304 user defined filepath
if err != nil {
return nil, fmt.Errorf("unable to read file at path %s: %w", path, err)
return nil, fmt.Errorf("glob path %s: %w", path, err)
}
if matches == nil {
return nil, fmt.Errorf("no matching files found when globbing %s", path)
}
defer f.Close()

resources := []*model.AnyResource{}
decoder := yaml.NewDecoder(f)
for {
resource := &model.AnyResource{}
if err := decoder.Decode(resource); err != nil {
if errors.Is(err, io.EOF) {
break

for _, match := range matches {
f, err := os.Open(match) // #nosec G304 user defined filepath
if err != nil {
return nil, fmt.Errorf("unable to read file at path %s: %w", path, err)
}
defer f.Close()

decoder := yaml.NewDecoder(f)
for {
resource := &model.AnyResource{}
if err := decoder.Decode(resource); err != nil {
if errors.Is(err, io.EOF) {
break
}
// TODO(jsirianni): Should we continue and report the error after?
return nil, fmt.Errorf("resource file %s is malformed, failed to unmarshal yaml: %w", path, err)
}
// TODO(jsirianni): Should we continue and report the error after?
return nil, fmt.Errorf("resource file %s is malformed, failed to unmarshal yaml: %w", path, err)
resources = append(resources, resource)
}
resources = append(resources, resource)
}

if len(resources) == 0 {
Expand Down
48 changes: 48 additions & 0 deletions action/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,3 +522,51 @@ func TestDecodeAnyResourceFile(t *testing.T) {
require.Contains(t, platforms, v, "Expected platform label to be one of %v, got %s", platforms, v)
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests failed before implementing the fix.

func TestDecodeAnyResourceFileGlob(t *testing.T) {
resources, err := decodeAnyResourceFile("testdata/*.yaml")
require.NoError(t, err)
require.NotNil(t, resources)
require.Len(t, resources, 4)

for _, r := range resources {
require.Equal(t, "bindplane.observiq.com/v1", r.APIVersion)
require.Equal(t, "Configuration", r.Kind)
require.NotEmpty(t, r.Metadata.ID)
require.NotEmpty(t, r.Metadata.Name)
require.Len(t, r.Metadata.Labels, 1, "Expected 1 label")
platforms := []string{
"kubernetes-gateway",
"kubernetes-daemonset",
"kubernetes-deployment",
}
key := "platform"
v, ok := r.Metadata.Labels[key]
require.True(t, ok, "Expected label %s", key)
require.Contains(t, platforms, v, "Expected platform label to be one of %v, got %s", platforms, v)
}
}

func TestDecodeAnyResourceFileGlobMatchOne(t *testing.T) {
resources, err := decodeAnyResourceFile("testdata/config*.yaml")
require.NoError(t, err)
require.NotNil(t, resources)
require.Len(t, resources, 3)

for _, r := range resources {
require.Equal(t, "bindplane.observiq.com/v1", r.APIVersion)
require.Equal(t, "Configuration", r.Kind)
require.NotEmpty(t, r.Metadata.ID)
require.NotEmpty(t, r.Metadata.Name)
require.Len(t, r.Metadata.Labels, 1, "Expected 1 label")
platforms := []string{
"kubernetes-gateway",
"kubernetes-daemonset",
"kubernetes-deployment",
}
key := "platform"
v, ok := r.Metadata.Labels[key]
require.True(t, ok, "Expected label %s", key)
require.Contains(t, platforms, v, "Expected platform label to be one of %v, got %s", platforms, v)
}
}
50 changes: 50 additions & 0 deletions action/testdata/cluster.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
apiVersion: bindplane.observiq.com/v1
kind: Configuration
metadata:
id: k8s-cluster2
name: k8s-cluster2
labels:
platform: kubernetes-deployment
spec:
contentType: ""
measurementInterval: ""
sources:
- id: 01HMS8GVNVFVD5TSWTKSJR1RY5
type: k8s_cluster
parameters:
- name: cluster_name
value: minikube
- name: node_conditions_to_report
value:
- Ready
- DiskPressure
- MemoryPressure
- PIDPressure
- NetworkUnavailable
- name: allocatable_types_to_report
value:
- cpu
- memory
- ephemeral-storage
- storage
- name: collection_interval
value: 60
- name: distribution
value: kubernetes
- id: 01HMS8GVNVFVD5TSWTKVNZS2JC
displayName: Production events
type: k8s_events
parameters:
- name: cluster_name
value: minikube
- name: namespaces
value:
- kube-system
- production
destinations:
- id: 01HMS8GVNVFVD5TSWTKZZNHB8R
name: bindplane-gateway-agent
selector:
matchLabels:
configuration: k8s-cluster2
30 changes: 15 additions & 15 deletions cmd/action/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import (
"fmt"
"net/url"
"os"
"path/filepath"

"github.com/observiq/bindplane-op-action/action"
"github.com/observiq/bindplane-op-action/internal/client/model"
)

func validate() error {
Expand Down Expand Up @@ -123,27 +125,25 @@ func validateActionsEnvironment() error {
}

func validateFilePaths() error {
if destination_path != "" {
if _, err := os.Stat(destination_path); os.IsNotExist(err) {
return fmt.Errorf("destination_path does not exist: %s", destination_path)
}
files := map[model.Kind]string{
model.KindDestination: destination_path,
model.KindSource: source_path,
model.KindProcessor: processor_path,
model.KindConfiguration: configuration_path,
}

if source_path != "" {
if _, err := os.Stat(source_path); os.IsNotExist(err) {
return fmt.Errorf("source_path does not exist: %s", source_path)
for kind, path := range files {
if path == "" {
continue
}
}

if processor_path != "" {
if _, err := os.Stat(processor_path); os.IsNotExist(err) {
return fmt.Errorf("processor_path does not exist: %s", processor_path)
matches, err := filepath.Glob(path)
if err != nil {
return fmt.Errorf("glob %s path %s: %w", kind, path, err)
}
}

if configuration_path != "" {
if _, err := os.Stat(configuration_path); os.IsNotExist(err) {
return fmt.Errorf("configuration_path does not exist: %s", configuration_path)
if matches == nil {
return fmt.Errorf("%s path %s does not exist or did not match any files with globbing", kind, path)
}
}

Expand Down
Loading