thought-machine / falco-probes

Automated build and mirror of eBPF kernel probes for use as a driver with the Falco runtime security agent (https://falco.org/)
Apache License 2.0
16 stars 4 forks source link

Implement IsAlreadyMirrored & cmd/is-falce-ebpf-probe-uploaded & add a temporary workflow to test #31

Closed lemanie closed 3 years ago

lemanie commented 3 years ago

I have a few questions about implementing this ticket:

  1. You mentioned I would need to use github actions on a feature branch instead of personal access token to test IsAlreadyMirrored, I'm not sure how that would work? Would we just add it in temporarily to test this ticket, then remove it during the next ticket where it gets used by the actual build action?
  2. I don't understand how the //cmd part can possibly work without using a) a personal access token b) an extra arg for using the repo.IsAlreadyMirrored?
  3. Even with a personal access token I'm getting a 401 with plz run //cmd/is-falco-ebpf-probe-uploaded -- --verbose --falco_version=0.28.1 --github_releases_token=$GRT amazonlinux2 4.14.200-155.322.amzn2 error="could not list releases: GET https://api.github.com/repos/thought-machine/falco-probes/releases: 401 Bad credentials []" I'm not sure if this is expected or something I need to configure on my github account?
  4. For IsAlreadyMirrored I added a returned error is that ok?
VJftw commented 3 years ago

I have a few questions about implementing this ticket:

  1. You mentioned I would need to use github actions on a feature branch instead of personal access token to test IsAlreadyMirrored, I'm not sure how that would work? Would we just add it in temporarily to test this ticket, then remove it during the next ticket where it gets used by the actual build action?

Yeah, that's exactly what I was thinking :) though, you're right this isn't a nice thing to do. Ideally we'd be writing unit tests to validate this which would get picked up by the "Test" github workflow. I started trying to get this working here: https://github.com/thought-machine/falco-probes/blob/unit-test-releases/pkg/repository/ghreleases/ghreleases_test.go#L31 but haven't quite got it yet. So i'm happy for any workarounds we have just to demonstrate that it works (i.e. pasting command output, or a temporary workflow).

  1. I don't understand how the //cmd part can possibly work without using a) a personal access token b) an extra arg for using the repo.IsAlreadyMirrored?

I'm not sure that I understand your question here; the //cmd/is-falco-ebpf-probe-uploaded's job is to check if the probe has already been uploaded. This requires a and is calling b. Ah right, sorry the command i've given you in your ticket doesn't detail the inputs. What you've done is 💯 what I was expecting 👍 i.e. plz run //cmd/is-falco-ebpf-probe-uploaded -- --falco_version <falco_version> <operating_system> <kernel_package_name>

  1. Even with a personal access token I'm getting a 401 with plz run //cmd/is-falco-ebpf-probe-uploaded -- --verbose --falco_version=0.28.1 --github_releases_token=$GRT amazonlinux2 4.14.200-155.322.amzn2 error="could not list releases: GET https://api.github.com/repos/thought-machine/falco-probes/releases: 401 Bad credentials []" I'm not sure if this is expected or something I need to configure on my github account?

Strange, it's working for me:

# Create github personal token from https://github.com/settings/tokens with no scopes attached
$ vim .github_token
# paste github token into file (can use nano)
$ export GITHUB_TOKEN=$(cat .github_token)
$ plz run //cmd/is-falco-ebpf-probe-uploaded -- --verbose --falco_version=0.29.1 amazonlinux2 4.14.200-155.322.amzn2
...
11:05AM INF Identifying if falco ebpf probe uploaded for  driverVersion=17f5df52a7d9ed6bb12d3b1768460def8439936d probeName=falco_amazonlinux2_4.14.200-155.322.amzn2.x86_64_1.o
11:05AM INF Matching release found, now checking it's assets:  release=17f5df52a7d9ed6bb12d3b1768460def8439936d
11:05AM FTL Probe cannot be found: error="release/asset not found that matches 17f5df52a7d9ed6bb12d3b1768460def8439936d/falco_amazonlinux2_4.14.200-155.322.amzn2.x86_64_1.o"
  1. For IsAlreadyMirrored I added a returned error is that ok?

Yup, that's okay! nice one 👍

lemanie commented 3 years ago

thanks @VJftw! I'll work on these changes now!

lemanie commented 3 years ago

Tested manually using: plz run //cmd/is-falco-ebpf-probe-uploaded -- --falco_version=0.29.1 amazonlinux2 4.14.200-155.322.amzn2 -> 1 plz run //cmd/is-falco-ebpf-probe-uploaded -- --falco_version=0.29.1 amazonlinux2 4.14.101-91.76.amzn2 -> 0

lemanie commented 3 years ago

Added in a workflow & accompanying build script:

These are a bit quick and dirty but we plan to delete them after the next ticket is completed where we will actually use IsAlreadyMirrored in the existing build script.

lemanie commented 3 years ago

2 test cases are still failing when they're meant to be passing, so I'm looking into it: Able to curl, but not able to find in workflow / in cmd:

lemanie commented 3 years ago

Added in pagination, tested locally using smaller PerPage values & running: ./pleasew -p -v2 run //build/github/test-is-already-mirrored -- amazonlinux2

lemanie commented 3 years ago

@VJftw one last question, I left the verifying input parts to the cmd not IsAlreadyMirrored. I thought it would be duplicating code when IsAlreadyMirrored is used properly, do you agree?

VJftw commented 3 years ago

@VJftw one last question, I left the verifying input parts to the cmd not IsAlreadyMirrored. I thought it would be duplicating code when IsAlreadyMirrored is used properly, do you agree?

Yes, there's very little value to verifying inputs as there are no negative consequences for bad inputs in the tooling we're making here. i.e. doing plz run //cmd/is-falco-ebpf-probe-uploaded -- amazonlinux2 as;djfhsladjfhj will just return an error saying that the package could not be downloaded 👍