-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from all commits
057be27
13f5067
ae63aae
66d3b28
b4a5d99
f9550a8
fcae47f
7e8edcc
51eadd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -241,7 +241,7 @@ func (a *Action) Apply() error { | |
} | ||
|
||
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) | ||
|
@@ -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) | ||
|
@@ -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) | ||
|
@@ -491,25 +491,41 @@ func (a *Action) WriteBack() error { | |
|
||
// decodeAnyResourceFile takes a file path and decodes it into a slice of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same logic, just inside a for loop iterating over |
||
// 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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} | ||
} |
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 |
There was a problem hiding this comment.
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.