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.

Sign up for GitHub

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

Refactor and enrich cloudbeat events #69

Merged
merged 17 commits into from
May 2, 2022
Merged

Refactor and enrich cloudbeat events #69

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b68dceb
add type and subtypes fields
uri-weisman Apr 26, 2022
4812158
commit before pr splits
uri-weisman Apr 27, 2022
e105383
add name field
uri-weisman May 1, 2022
77a443a
Merge branch 'main' into refactor_resource_struct
uri-weisman May 1, 2022
73adf59
unit tests + csp policies repo tag change
uri-weisman May 1, 2022
6cf20f5
update go.mod
uri-weisman May 1, 2022
322e71f
Update JUSTFILE
uri-weisman May 1, 2022
a7b24d1
Update JUSTFILE
uri-weisman May 1, 2022
5e80a03
revert justfile changes
uri-weisman May 1, 2022
f4b13e5
Merge branch 'refactor_resource_struct' of https://github.com/uri-wei…
uri-weisman May 1, 2022
b35a8fd
prepare pr for review
uri-weisman May 1, 2022
4b2373a
update integration tests with new types
uri-weisman May 1, 2022
43baff4
Merge branch 'main' into refactor_resource_struct
uri-weisman May 1, 2022
d1dbb9c
CR changes
uri-weisman May 1, 2022
ebdb3f1
Merge branch 'refactor_resource_struct' of https://github.com/uri-wei…
uri-weisman May 1, 2022
366d31c
Merge branch 'main' into refactor_resource_struct
uri-weisman May 1, 2022
2905f65
Merge branch 'main' into refactor_resource_struct
uri-weisman May 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion JUSTFILE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,4 @@ gen-report:
allure generate tests/allure/results --clean -o tests/allure/reports && cp tests/allure/reports/history/* tests/allure/results/history/. && allure open tests/allure/reports

run-tests:
helm test cloudbeat-tests --namespace kube-system
helm test cloudbeat-tests --namespace kube-system
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ require (
github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect
github.com/eapache/queue v1.1.0 // indirect
github.com/elastic/beats/v7 v7.0.0-alpha2.0.20220413140705-d101ba1d2ae5
github.com/elastic/csp-security-policies v0.0.14-go-lib
github.com/elastic/csp-security-policies v0.0.15-go-lib
github.com/ghodss/yaml v1.0.0 // indirect
github.com/go-ini/ini v1.66.4 // indirect
github.com/go-logr/logr v1.2.2 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,10 @@ github.com/elastic/beats/v7 v7.0.0-alpha2.0.20220413140705-d101ba1d2ae5 h1:kINUJ
github.com/elastic/beats/v7 v7.0.0-alpha2.0.20220413140705-d101ba1d2ae5/go.mod h1:PhsCH91qJN33+rN/L8q2jWILmswlezJ6T+MMM6EDc8g=
github.com/elastic/csp-security-policies v0.0.14-go-lib h1:B/Ylu4aLhfcOcM3YivzwMd4BB/NoT/jFIV98fXtZDVA=
github.com/elastic/csp-security-policies v0.0.14-go-lib/go.mod h1:24NNr0b/5HTGtndJOmhrefb59rd7NjuqI/To39tgn+w=
github.com/elastic/csp-security-policies v0.0.15-go-lib h1:lLmtmmL9Gngo8F8y0K1dwA6UW00sfIW3zq4t8CjuHkU=
github.com/elastic/csp-security-policies v0.0.15-go-lib/go.mod h1:24NNr0b/5HTGtndJOmhrefb59rd7NjuqI/To39tgn+w=
github.com/elastic/csp-security-policies v0.0.15-go-lib-alpha h1:hiVX1rm/jnBt7eiSQkDo8xTx6mF8kyg1kQpnnc21zpI=
github.com/elastic/csp-security-policies v0.0.15-go-lib-alpha/go.mod h1:24NNr0b/5HTGtndJOmhrefb59rd7NjuqI/To39tgn+w=
github.com/elastic/elastic-agent-client/v7 v7.0.0-20210727140539-f0905d9377f6 h1:nFvXHBjYK3e9+xF0WKDeAKK4aOO51uC28s+L9rBmilo=
github.com/elastic/elastic-agent-client/v7 v7.0.0-20210727140539-f0905d9377f6/go.mod h1:uh/Gj9a0XEbYoM4NYz4LvaBVARz3QXLmlNjsrKY9fTc=
github.com/elastic/elastic-agent-libs v0.1.1/go.mod h1:h8K/f7RcdxM2f19VahcS1jeco170ItqV9N7HyYsn9Ss=
Expand Down
15 changes: 11 additions & 4 deletions resources/fetchers/ecr_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,17 @@ func (f *ECRFetcher) getAwsPodRepositories(ctx context.Context) ([]string, error
return repositories, nil
}

// GetID TODO: Add resource id logic to all AWS resources
func (res ECRResource) GetID() (string, error) {
return "", nil
}
func (res ECRResource) GetData() interface{} {
//TODO implement me
return res
}

func (res ECRResource) GetMetadata() fetching.ResourceMetadata {
//TODO implement me
Copy link
Contributor

Choose a reason for hiding this comment

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

please open an issue for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A task for implementing the missing GetMetadata method - #71

return fetching.ResourceMetadata{
ID: "",
Type: "",
SubType: "",
Name: "",
}
}
15 changes: 10 additions & 5 deletions resources/fetchers/eks_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,16 @@ func (f EKSFetcher) Fetch(ctx context.Context) ([]fetching.Resource, error) {
func (f EKSFetcher) Stop() {
}

// GetID TODO: Add resource id logic to all AWS resources
func (r EKSResource) GetID() (string, error) {
return "", nil
}

func (r EKSResource) GetData() interface{} {
return r
}

func (r EKSResource) GetMetadata() fetching.ResourceMetadata {
//TODO implement me
return fetching.ResourceMetadata{
Copy link
Contributor

Choose a reason for hiding this comment

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

in the issue you open include a list of fetchers to fix

ID: "",
Type: "",
SubType: "",
Name: "",
}
}
15 changes: 10 additions & 5 deletions resources/fetchers/elb_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,16 @@ func (f *ELBFetcher) GetLoadBalancers() ([]string, error) {
func (f *ELBFetcher) Stop() {
}

// GetID TODO: Add resource id logic to all AWS resources
func (r ELBResource) GetID() (string, error) {
return "", nil
}

func (r ELBResource) GetData() interface{} {
return r
}

func (r ELBResource) GetMetadata() fetching.ResourceMetadata {
//TODO implement me
return fetching.ResourceMetadata{
ID: "",
Type: "",
SubType: "",
Name: "",
}
}
26 changes: 21 additions & 5 deletions resources/fetchers/file_system_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@ import (
"github.com/elastic/cloudbeat/resources/fetching"
)

const FSResourceType = "file"

type FileSystemResource struct {
FileName string `json:"filename"`
FileMode string `json:"mode"`
Gid string `json:"gid"`
Uid string `json:"uid"`
Path string `json:"path"`
Inode string `json:"inode"`
SubType string `json:"sub_type"`
}

// FileSystemFetcher implement the Fetcher interface
Expand Down Expand Up @@ -104,7 +107,7 @@ func FromFileInfo(info os.FileInfo, path string) (FileSystemResource, error) {
usr, _ := user.LookupId(u)
group, _ := user.LookupGroupId(g)
mod := strconv.FormatUint(uint64(info.Mode().Perm()), 8)
inode := strconv.FormatUint(uint64(stat.Ino), 10)
uri-weisman marked this conversation as resolved.
Show resolved Hide resolved
inode := strconv.FormatUint(stat.Ino, 10)

data := FileSystemResource{
FileName: info.Name(),
Expand All @@ -113,6 +116,7 @@ func FromFileInfo(info os.FileInfo, path string) (FileSystemResource, error) {
Gid: group.Name,
Path: path,
Inode: inode,
SubType: getSubType(info.IsDir()),
}

return data, nil
Expand All @@ -121,10 +125,22 @@ func FromFileInfo(info os.FileInfo, path string) (FileSystemResource, error) {
func (f *FileSystemFetcher) Stop() {
}

func (r FileSystemResource) GetID() (string, error) {
return r.Inode, nil
}

func (r FileSystemResource) GetData() interface{} {
return r
}

func (r FileSystemResource) GetMetadata() fetching.ResourceMetadata {
return fetching.ResourceMetadata{
ID: r.Inode,
Type: FSResourceType,
SubType: r.SubType,
Name: r.Path,
Copy link
Contributor

Choose a reason for hiding this comment

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

leave a note about this being the path from the container and not from the host

}
}

func getSubType(isDir bool) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

that's a bit weird.
what about passing info?

if isDir {
return "directory"
}
return "file"
}
55 changes: 36 additions & 19 deletions resources/fetchers/file_system_fetcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ func TestFileFetcherFetchASingleFile(t *testing.T) {
fsResource := results[0].(FileSystemResource)
assert.Equal(t, files[0], fsResource.FileName)
assert.Equal(t, "600", fsResource.FileMode)
rid, err := fsResource.GetID()
assert.NotNil(t, rid)
assert.NoError(t, err)

rMetadata := fsResource.GetMetadata()
assert.NotNil(t, rMetadata.ID)
assert.Equal(t, filePaths[0], rMetadata.Name)
assert.Equal(t, "file", rMetadata.SubType)
assert.Equal(t, FSResourceType, rMetadata.Type)
}

func TestFileFetcherFetchTwoPatterns(t *testing.T) {
Expand All @@ -59,9 +62,9 @@ func TestFileFetcherFetchTwoPatterns(t *testing.T) {
outerDir := createDirectoriesWithFiles(t, "", outerDirectoryName, outerFiles)
defer os.RemoveAll(outerDir)

path := []string{filepath.Join(outerDir, outerFiles[0]), filepath.Join(outerDir, outerFiles[1])}
paths := []string{filepath.Join(outerDir, outerFiles[0]), filepath.Join(outerDir, outerFiles[1])}
cfg := FileFetcherConfig{
Patterns: path,
Patterns: paths,
}
factory := FileSystemFactory{}
fileFetcher, err := factory.CreateFrom(cfg)
Expand All @@ -74,16 +77,22 @@ func TestFileFetcherFetchTwoPatterns(t *testing.T) {
firstFSResource := results[0].(FileSystemResource)
assert.Equal(t, outerFiles[0], firstFSResource.FileName)
assert.Equal(t, "600", firstFSResource.FileMode)
rid, err := firstFSResource.GetID()
assert.NotNil(t, rid)
assert.NoError(t, err)

rMetadata := firstFSResource.GetMetadata()
assert.NotNil(t, rMetadata.ID)
assert.Equal(t, paths[0], rMetadata.Name)
assert.Equal(t, "file", rMetadata.SubType)
assert.Equal(t, FSResourceType, rMetadata.Type)

secFSResource := results[1].(FileSystemResource)
assert.Equal(t, outerFiles[1], secFSResource.FileName)
assert.Equal(t, "600", secFSResource.FileMode)
rid, err = secFSResource.GetID()
assert.NotNil(t, rid)
assert.NoError(t, err)

SecResMetadata := secFSResource.GetMetadata()
assert.NotNil(t, SecResMetadata.ID)
assert.Equal(t, paths[1], SecResMetadata.Name)
assert.Equal(t, "file", SecResMetadata.SubType)
assert.Equal(t, FSResourceType, SecResMetadata.Type)
}

func TestFileFetcherFetchDirectoryOnly(t *testing.T) {
Expand All @@ -105,12 +114,14 @@ func TestFileFetcherFetchDirectoryOnly(t *testing.T) {
assert.Equal(t, 1, len(results))

fsResource := results[0].(FileSystemResource)

expectedResult := filepath.Base(dir)
rMetadata := fsResource.GetMetadata()

assert.Equal(t, expectedResult, fsResource.FileName)
rid, err := fsResource.GetID()
assert.NotNil(t, rid)
assert.NoError(t, err)
assert.NotNil(t, rMetadata.ID)
assert.NotNil(t, rMetadata.Name)
assert.Equal(t, "directory", rMetadata.SubType)
assert.Equal(t, FSResourceType, rMetadata.Type)
}

func TestFileFetcherFetchOuterDirectoryOnly(t *testing.T) {
Expand Down Expand Up @@ -138,10 +149,13 @@ func TestFileFetcherFetchOuterDirectoryOnly(t *testing.T) {
//All inner files should exist in the final result
expectedResult := []string{"output.txt", filepath.Base(innerDir)}
for i := 0; i < len(results); i++ {
rMetadata := results[i].GetMetadata()
fileSystemDataResources := results[i].(FileSystemResource)
assert.Contains(t, expectedResult, fileSystemDataResources.FileName)
rid, err := results[i].GetID()
assert.NotNil(t, rid)
assert.NotNil(t, rMetadata.SubType)
assert.NotNil(t, rMetadata.Name)
assert.NotNil(t, rMetadata.ID)
assert.Equal(t, FSResourceType, rMetadata.Type)
assert.NoError(t, err)
}
}
Expand Down Expand Up @@ -178,8 +192,11 @@ func TestFileFetcherFetchDirectoryRecursively(t *testing.T) {
//All inner files should exist in the final result
for i := 0; i < len(results); i++ {
fileSystemDataResources := results[i].(FileSystemResource)
rid, err := results[i].GetID()
assert.NotNil(t, rid)
rMetadata := results[i].GetMetadata()
assert.NotNil(t, rMetadata.SubType)
assert.NotNil(t, rMetadata.Name)
assert.NotNil(t, rMetadata.ID)
assert.Equal(t, FSResourceType, rMetadata.Type)
assert.NoError(t, err)
assert.Contains(t, allFilesName, fileSystemDataResources.FileName)
}
Expand Down
10 changes: 10 additions & 0 deletions resources/fetchers/iam_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,13 @@ func (r IAMResource) GetID() (string, error) {
func (r IAMResource) GetData() interface{} {
return r.Data
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You did not remove GetID() in iam_fetcher.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks 💪

func (r IAMResource) GetMetadata() fetching.ResourceMetadata {
//TODO implement me
return fetching.ResourceMetadata{
ID: "",
Type: "",
SubType: "",
Name: "",
}
}
42 changes: 34 additions & 8 deletions resources/fetchers/kube_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package fetchers

import (
"fmt"
"reflect"

"github.com/elastic/beats/v7/libbeat/common/kubernetes"
Expand All @@ -31,7 +30,11 @@ type K8sResource struct {
Data interface{}
}

const k8sObjMetadataField = "ObjectMeta"
const (
k8sObjMetadataField = "ObjectMeta"
k8sTypeMetadataField = "TypeMeta"
k8sObjType = "k8s_object"
)

func GetKubeData(watchers []kubernetes.Watcher) []fetching.Resource {
logp.L().Info("Fetching Kubernetes data")
Expand Down Expand Up @@ -62,19 +65,42 @@ func GetKubeData(watchers []kubernetes.Watcher) []fetching.Resource {
return ret
}

func (r K8sResource) GetID() (string, error) {
func (r K8sResource) GetData() interface{} {
return r.Data
}

func (r K8sResource) GetMetadata() fetching.ResourceMetadata {
k8sObjMeta := r.GetK8sObjectMeta()
resourceID := k8sObjMeta.UID
resourceName := k8sObjMeta.Name

return fetching.ResourceMetadata{
ID: string(resourceID),
Type: k8sObjType,
SubType: r.GetSubType(),
Name: resourceName,
}
}

func (r K8sResource) GetK8sObjectMeta() metav1.ObjectMeta {
k8sObj := reflect.Indirect(reflect.ValueOf(r.Data))
metadata, ok := k8sObj.FieldByName(k8sObjMetadataField).Interface().(metav1.ObjectMeta)
if !ok {
return "", fmt.Errorf("failed to retrieve object metadata")
logp.L().Errorf("failed to retrieve object metadata, Resource: %#v", r)
return metav1.ObjectMeta{}
}

uid := metadata.UID
return string(uid), nil
return metadata
}

func (r K8sResource) GetData() interface{} {
return r.Data
func (r K8sResource) GetSubType() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to fail and continue with an empty string value instead of the kind?
Just wondering if we are interested in sending data without kind that might create problems down the line of the pipeline that expects to groupBy or do any logic on these values?

Copy link
Contributor

Choose a reason for hiding this comment

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

good question👆🏼
I think i'd prefer events with subtype missing than no events at all,
but the possibility for problems in Kibana resulting from this field missing is interesting...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think sending with empty subtype matches the current design pattern and is a good start (best effort).
for example currently if a fetcher registration fails cloudbeat will log an error and continue.
anyways it's a good idea to ask @kfirpeled / @ari-aviran or someone else from the FE, or even comment here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, except for bugs I don't see any reason for this method ever failing so I wouldn't waste too much time thinking/debating it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats a good point,
In case of an issue - we can define it to be empty. And we would know to treat empty values as error.

What happens when there's no subtype? For example, in process.
Just to make things easier, I think it is better to have value we would like to see. And not solve this in the UI only or assume if it is process there's no error and it is empty. Just so we can allow our users to built their own dashboard over the data without having some sort of logic.

k8sObj := reflect.Indirect(reflect.ValueOf(r.Data))
Copy link
Contributor

Choose a reason for hiding this comment

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

try and redesign to avoid calling reflect.Indirect(reflect.ValueOf(r.Data)) twice

typeMeta, ok := k8sObj.FieldByName(k8sTypeMetadataField).Interface().(metav1.TypeMeta)
if !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

now i see what you meant...

logp.L().Errorf("failed to retrieve type metadata, Resource: %#v", r)
}

return typeMeta.Kind
}

// nullifyManagedFields ManagedFields field contains fields with dot that prevent from elasticsearch to index
Expand Down
17 changes: 12 additions & 5 deletions resources/fetchers/process_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ const (
// Expects format as the following: --<key><delimiter><value>.
// For example: --config=a.json
// The regex supports two delimiters "=" and ""
CMDArgumentMatcher = "\\b%s[\\s=]\\/?(\\S+)"
CMDArgumentMatcher = "\\b%s[\\s=]\\/?(\\S+)"
ProcessResourceType = "process"
ProcessSubType = "process"
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

)

type ProcessResource struct {
Expand Down Expand Up @@ -163,10 +165,15 @@ func (f *ProcessesFetcher) readConfigurationFile(path string, data []byte) (inte
func (f *ProcessesFetcher) Stop() {
}

func (res ProcessResource) GetID() (string, error) {
return res.PID, nil
}

func (res ProcessResource) GetData() interface{} {
return res
}

func (res ProcessResource) GetMetadata() fetching.ResourceMetadata {
return fetching.ResourceMetadata{
ID: res.PID,
Type: ProcessResourceType,
SubType: ProcessSubType,
Name: res.Stat.Name,
}
}
13 changes: 12 additions & 1 deletion resources/fetching/fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,21 @@ type Condition interface {
}

type Resource interface {
GetID() (string, error)
GetMetadata() ResourceMetadata
GetData() interface{}
}

type ResourceFields struct {
ResourceMetadata
Raw interface{} `json:"raw"`
}

type ResourceMetadata struct {
ID string `json:"id"`
Type string `json:"type"`
SubType string `json:"sub_type"`
Name string `json:"name"`
}
type Result struct {
Type string `json:"type"`
// Golang 1.18 will introduce generics which will be useful for typing the resource field
Expand Down
Loading