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

feat(integrations): add the falco integrations from the main falco repo #3

Merged
merged 19 commits into from
Apr 27, 2020

Conversation

maxgio92
Copy link
Member

start to move the non-official and community maintained integrations of falco from the main
repository.

@maxgio92 maxgio92 linked an issue Apr 18, 2020 that may be closed by this pull request
@maxgio92 maxgio92 self-assigned this Apr 18, 2020
start to move the non-official and community maintained integrations of falco from the main
repository.

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
@maxgio92 maxgio92 force-pushed the feature/integrations branch from 3e74b64 to b64b152 Compare April 18, 2020 16:28
@maxgio92
Copy link
Member Author

/cc @leodido

@leogr
Copy link
Member

leogr commented Apr 18, 2020

LGTM!

@leodido
Copy link
Member

leodido commented Apr 18, 2020

/check-dco

### Build the image

```
docker build -t sysdig/anchore-falco .
Copy link
Member

Choose a reason for hiding this comment

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

Can we please fix such occurrences? :)

Copy link
Member Author

@maxgio92 maxgio92 Apr 19, 2020

Choose a reason for hiding this comment

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

If it's what you mean, I didn't find a Docker hub repository in the falcosecurity organization as for the event-generator. It should still be created, right?

Copy link
Member

Choose a reason for hiding this comment

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

Since there are no other references to sysdig/anchore-falco besides these in the README, I would suggest to just rewrote the instruction in order to use a local image (eg. anchore-falco).

IMHO, it would be enough. If it was actively maintained and required a new repo, we will create it later.

docker run --rm -e ANCHORE_CLI_USER=<user-for-custom-anchore-engine> \
-e ANCHORE_CLI_PASS=<password-for-user-for-custom-anchore-engine> \
-e ANCHORE_CLI_URL=http://<custom-anchore-engine-host>:8228/v1 \
sysdig/anchore-falco
Copy link
Member

Choose a reason for hiding this comment

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

And here!

Copy link
Member Author

Choose a reason for hiding this comment

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

The same as above

spec:
containers:
- name: falco-event-generator
image: sysdig/falco-event-generator:latest
Copy link
Member

Choose a reason for hiding this comment

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

This needs probably to be:

  1. renamed
  2. coordinate with the new event-generator at https://github.com/falcosecurity/event-generator

Copy link
Member Author

@maxgio92 maxgio92 Apr 19, 2020

Choose a reason for hiding this comment

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

I think that it can be considered done :)

Copy link
Member

Choose a reason for hiding this comment

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

Atm, falcosecurity/event-generator is not yet ready, although a test image already exists.

@leodido WDYT ?
We may keep this on hold until dependant issues are solved. Regarding the event-generator, I guess it will take another week or so.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@leogr we'll keep the sysdig one for now? Or we can wait for a release of the Docker image v0.1.0? Thank you!

Copy link
Member

@leogr leogr Apr 23, 2020

Choose a reason for hiding this comment

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

You are welcome!
It's released on Docker Hub too, see my review below

# - name: FALCOCTL_FALCO_PROBE_URL
# value:
# - name: FALCOCTL_FALCO_PROBE_REPO
# value: "https://s3.amazonaws.com/download.draios.com/stable/sysdig-probe-binaries/"
Copy link
Member

Choose a reason for hiding this comment

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

This URL can be left as is for now (but it'll need to be updated soon :))

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure those are working, I propose to fix this later in another PR and unlock this now since we have everything else solved (once suggestions have been applied).
@leodido @maxgio92 WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree @leogr

Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

Some changes about names, nothing serious :)

Thanks for helping us Max! 🤗

update integrations' version with that of falco's master branch.

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
@maxgio92
Copy link
Member Author

Sure @leodido ! I just replied to the specific reviews for clarifications and updated this branch because it was out-of-sync with the Falco repo's master branch.
Thank you

…itory

replace the sysdig organization with the falcosecurity one

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
…odes

sync kubernetes deployment and jobs specs from the event-generator git repository

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

Please, check out my suggestions!

Thank you!

integrations/anchore-falco/README.md Outdated Show resolved Hide resolved
integrations/anchore-falco/README.md Outdated Show resolved Hide resolved
integrations/anchore-falco/README.md Outdated Show resolved Hide resolved
integrations/k8s-using-daemonset/README.md Outdated Show resolved Hide resolved
integrations/k8s-using-daemonset/README.md Outdated Show resolved Hide resolved
integrations/k8s-using-daemonset/README.md Outdated Show resolved Hide resolved
integrations/k8s-using-daemonset/README.md Outdated Show resolved Hide resolved
maxgio92 and others added 4 commits April 25, 2020 15:33
remove references to remote image published on docker hub

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
remove references to sysdig remote repository

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
remove references to remote image published on docker hub

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
- run only syscall, otherwise also the k8saudit events collection will run
- run inside loop

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
@maxgio92 maxgio92 force-pushed the feature/integrations branch from 6a739f0 to be6eb63 Compare April 25, 2020 13:51
maxgio92 and others added 10 commits April 25, 2020 15:52
- run only syscall, otherwise also the k8saudit events collection will run

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
- run only syscall, otherwise also the k8saudit events collection will run
- run inside loop

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
- run only syscall, otherwise also the k8saudit events collection will run

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
set the event-generator github repo https url

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
refer to the official documentation for samples, remove duplicates

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
refer to the official documentation for samples, remove duplicates

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
refer to the official documentation for samples, remove duplicates

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>

Co-Authored-By: Leonardo Grasso <me@leonardograsso.com>
remove references to local kubernetes config samples, instead refer to the official repo

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
remove references to local kubernetes config samples, instead refer
to the official repo

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
remove kubernetes event-generator config samples from falco k8s
deployment integration, instead refer to the official repo

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
@maxgio92 maxgio92 force-pushed the feature/integrations branch from 054d1cf to 4c35201 Compare April 25, 2020 14:37
remove kubernetes event-generator config samples from falco k8s
daemonset integration, instead refer to the official repo

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
@maxgio92
Copy link
Member Author

Just minor fixes to the event-generator stuff.

Btw, I think we could move the event-generator related files to the new event-generator repo and just link them in the documentation here. That would help with the maintenance.

Please, feel free to open a new PR in the new event-generator repo and clean them up from here.

Thanks again!

Hi @leogr, I implemented the suggestions and then cleaned the event-generator sample configurations for consistency; updated the documentation to refer to the new official event-generator's repo.

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM now

thanks @maxgio92

@poiana poiana added the lgtm label Apr 26, 2020
@poiana
Copy link
Contributor

poiana commented Apr 26, 2020

LGTM label has been added.

Git tree hash: 6c0e736ff8c61472b935c1592f08ba12148ea2c1

@leodido leodido self-requested a review April 27, 2020 09:49
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

Approving to unblock and merge it.

FYI the initcontainer in the k8s examples should be updated.

@poiana
Copy link
Contributor

poiana commented Apr 27, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leodido, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 64ee514 into master Apr 27, 2020
@poiana poiana deleted the feature/integrations branch April 27, 2020 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move here the existing integrations from the main Falco repository
4 participants