Reposupport #69

Atul-source · 2022-06-06T10:10:37Z

This PR is for issue #18

Before you could only have a single repository defined for all eBPF programs in the global config. This change enables the ability to define an eBPF repository per program.

satyaranjanp · 2022-06-08T06:08:23Z

models/l3afd.go

+	MapArgs           L3afDNFArgs         `json:"map_args"`               // Config BPF Map of arguments
+	ConfigArgs        L3afDNFArgs         `json:"config_args"`            // Map of arguments to config command
+	MonitorMaps       []L3afDNFMetricsMap `json:"monitor_maps"`           // Metrics BPF maps
+	DownloadUrl       string              `json:"ebpf_program_repo_name"` // Download url for Program


Can we rename the variable to something more related to eBPF Package repository, for ex. EPRUrl. Also, can we have the json tag as ebpf_package_repo_url

satyaranjanp · 2022-06-08T06:08:23Z

models/l3afd.go

+	MapArgs           L3afDNFArgs         `json:"map_args"`               // Config BPF Map of arguments
+	ConfigArgs        L3afDNFArgs         `json:"config_args"`            // Map of arguments to config command
+	MonitorMaps       []L3afDNFMetricsMap `json:"monitor_maps"`           // Metrics BPF maps
+	DownloadUrl       string              `json:"ebpf_program_repo_name"` // Download url for Program


Can we rename the variable to something more related to eBPF Package repository, for ex. EPRUrl. Also, can we have the json tag as ebpf_package_repo_url

I have changed variables names

dthaler

Thanks for doing this! Changes look good so far.

https://github.com/l3af-project/l3afd/blob/main/docs/CONTRIBUTING.md#guidelines-for-pull-requests says:

Describe the tests that you ran to verify your changes and add/update test cases
Update relevant docs, especially if you've changed APIs

and also

PRs are expected to have 100% test coverage for added code. This can be verified with a coverage build. If your PR cannot have 100% coverage for some reason please clearly explain why when you open it.

What about documentation changes for this?
What about tests that verify that local files work? (PR #66 added some tests, but since this PR is separate I assume that PR doesn't cover tests for this PR)

dthaler · 2022-06-08T14:18:11Z

kf/bpf.go

+	//      "file://" ---> has size 7
+	//     "Http://" ---> has size 7


Suggested change

// "file://" ---> has size 7

// "Http://" ---> has size 7

// "file://" ---> has size 7

// "http://" ---> has size 7

sanfern · 2022-06-08T18:14:58Z

@Atul-source Can you update api documentation https://github.com/l3af-project/l3afd/blob/main/docs/api/README.md

jniesz · 2022-06-14T14:43:51Z

docs/api/README.md

@@ -15,6 +15,7 @@ The payload will look more like this standard JSON:
          "name": "ratelimiting",
          "seq_id": 1,
          "artifact": "l3af_ratelimiting.tar.gz",
+          "ebpf_package_repo_url": "http://www.exampleebpf.com/l3af_ratelimiting.tar.gz"


Let's put example URL of domain we own, such as l3af.io and use https as an example

jniesz · 2022-06-14T15:02:55Z

kf/bpf.go

+	buf := &bytes.Buffer{}
+	//      "file://" ---> has size 7
+	//     "http://" ---> has size 7
+	if len(b.Program.EPRUrl) >= 7 {


We should just use url.parse to determine the scheme (http, https, file, ...). File should have three slashes "file:///"

docs/api/README.md

kf/bpf.go

jniesz

Left some comments for changes

kf/bpf.go

jniesz · 2022-06-17T18:04:45Z

kf/bpf.go

+			}
+		}
+	} else {
+		kfRepoURL, err := url.Parse(conf.KFRepoURL)


Future PR we should rename this to eBPFRepoURL

I will do a separate PR for a change of KFRepoURL --> eBPFRepoURL

kf/bpf.go

jniesz · 2022-06-17T20:34:34Z

kf/bpf.go

-			if strings.Contains(header.Name, "..") {
-				return fmt.Errorf("zipped file contians filepath (%s) that includes (..)", header.Name)
-			}
+				if strings.Contains(header.Name, "..") {


Can we have a similar check as the zip filename check above with filepath.Clean() ? It probably also makes breaking out this code, so as we add support for new compression algorithms the filename checking logic remains the same and can be managed in single place.

if we check for somewhere else using the helper function it is giving security issues.

breaking out into a separate function vs. having similar code 2+ more times seems to make sense, not sure why this is alerting if we make this change. We can discuss more on call

…positories Signed-off-by: a0p0ie5 <[email protected]>

Signed-off-by: a0p0ie5 <[email protected]>

Co-authored-by: Dave Thaler <[email protected]> Signed-off-by: a0p0ie5 <[email protected]>

Co-authored-by: jniesz <[email protected]> Signed-off-by: a0p0ie5 <[email protected]>

Signed-off-by: a0p0ie5 <[email protected]>

docs/api/README.md

Signed-off-by: a0p0ie5 <[email protected]>

…to reposupport

jniesz

LGTM

dthaler · 2022-06-22T16:18:09Z

Changes look good, but needs a test added per https://github.com/l3af-project/l3afd/blob/main/docs/CONTRIBUTING.md#guidelines-for-pull-requests which says:

Describe the tests that you ran to verify your changes and add/update test cases
Update relevant docs, especially if you've changed APIs

and also

PRs are expected to have 100% test coverage for added code. This can be verified with a coverage build. If your PR cannot have 100% coverage for some reason please clearly explain why when you open it.

Signed-off-by: a0p0ie5 <[email protected]>

kf/bpf_test.go

Atul-source requested review from sanfern, satyaranjanp, jniesz and dthaler as code owners June 6, 2022 10:10

Atul-source force-pushed the reposupport branch from f51dd3b to 56f9543 Compare June 6, 2022 10:11

satyaranjanp reviewed Jun 8, 2022

View reviewed changes

dthaler reviewed Jun 8, 2022

View reviewed changes

jniesz reviewed Jun 14, 2022

View reviewed changes

dthaler reviewed Jun 17, 2022

View reviewed changes

docs/api/README.md Outdated Show resolved Hide resolved

kf/bpf.go Outdated Show resolved Hide resolved

jniesz reviewed Jun 17, 2022

View reviewed changes

a0p0ie5 and others added 12 commits June 18, 2022 10:59

Add support for Artifact Download from local repo and other online re…

41f50e9

…positories Signed-off-by: a0p0ie5 <[email protected]>

changed variable name

96a98af

Signed-off-by: a0p0ie5 <[email protected]>

Local repo and other online repo support for Download

686d3fa

Signed-off-by: a0p0ie5 <[email protected]>

changed varible names

b7ad166

Signed-off-by: a0p0ie5 <[email protected]>

l3afd/doc/api/README.md updated

6d6c8b1

Signed-off-by: a0p0ie5 <[email protected]>

comment changes

113ef1e

Signed-off-by: a0p0ie5 <[email protected]>

some code cleaning and docs/api/README.md updated

f4d2d81

Signed-off-by: a0p0ie5 <[email protected]>

replaced multiple if-else with switch

7913b96

Signed-off-by: a0p0ie5 <[email protected]>

updated the doc

f9ffeac

Signed-off-by: a0p0ie5 <[email protected]>

Update docs/api/README.md

ce1d173

Co-authored-by: Dave Thaler <[email protected]> Signed-off-by: a0p0ie5 <[email protected]>

Update kf/bpf.go

885ed7d

Co-authored-by: Dave Thaler <[email protected]> Signed-off-by: a0p0ie5 <[email protected]>

Update kf/bpf.go

8dc8135

Co-authored-by: jniesz <[email protected]> Signed-off-by: a0p0ie5 <[email protected]>

Atul-source force-pushed the reposupport branch from e8f4c18 to 8dc8135 Compare June 18, 2022 05:29

a0p0ie5 added 4 commits June 18, 2022 12:26

changes according to the reviews

cc67c49

Signed-off-by: a0p0ie5 <[email protected]>

resolved security issue

31aa153

Signed-off-by: a0p0ie5 <[email protected]>

validated function

2e567ef

Signed-off-by: a0p0ie5 <[email protected]>

Merge branch 'main' into reposupport

fe8d15d

dthaler reviewed Jun 20, 2022

View reviewed changes

docs/api/README.md Outdated Show resolved Hide resolved

a0p0ie5 added 2 commits June 21, 2022 09:48

fixed the doc

012f877

Signed-off-by: a0p0ie5 <[email protected]>

Merge branch 'reposupport' of https://github.com/Atul-source/l3afd in…

9cf4765

…to reposupport

Atul-source force-pushed the reposupport branch from 0a57785 to 9cf4765 Compare June 21, 2022 04:26

jniesz approved these changes Jun 21, 2022

View reviewed changes

a0p0ie5 added 7 commits June 23, 2022 11:49

added tests

d495bf2

Signed-off-by: a0p0ie5 <[email protected]>

tests

370f5a7

Signed-off-by: a0p0ie5 <[email protected]>

some typo correction

c4b79ac

Signed-off-by: a0p0ie5 <[email protected]>

added some mocks

8985195

Signed-off-by: a0p0ie5 <[email protected]>

resolving merge conflict

c00e56d

Signed-off-by: a0p0ie5 <[email protected]>

resolving conflict

52223d8

Signed-off-by: a0p0ie5 <[email protected]>

removed unused file

8bed18f

Signed-off-by: a0p0ie5 <[email protected]>

dthaler approved these changes Jul 6, 2022

View reviewed changes

kf/bpf_test.go Outdated Show resolved Hide resolved

Update kf/bpf_test.go

cf51871

sanfern approved these changes Jul 6, 2022

View reviewed changes

dthaler merged commit 3c8f904 into l3af-project:main Jul 7, 2022

Atul-source deleted the reposupport branch August 18, 2022 06:56

jniesz added the feat New Feature label Nov 3, 2022

jniesz mentioned this pull request Nov 3, 2022

Add TLS support for client that downloads eBPF programs #2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reposupport #69

Reposupport #69

Atul-source commented Jun 6, 2022 •

edited by jniesz

Loading

satyaranjanp Jun 8, 2022

satyaranjanp Jun 8, 2022

Atul-source Jun 8, 2022

dthaler left a comment •

edited

Loading

dthaler Jun 8, 2022

sanfern commented Jun 8, 2022

jniesz Jun 14, 2022 •

edited

Loading

jniesz Jun 14, 2022

jniesz left a comment

jniesz Jun 17, 2022

Atul-source Jun 18, 2022

jniesz Jun 17, 2022

Atul-source Jun 18, 2022

jniesz Jun 20, 2022

jniesz left a comment

dthaler commented Jun 22, 2022

Reposupport #69

Reposupport #69

Conversation

Atul-source commented Jun 6, 2022 • edited by jniesz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dthaler left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanfern commented Jun 8, 2022

jniesz Jun 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jniesz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jniesz left a comment

Choose a reason for hiding this comment

dthaler commented Jun 22, 2022

Atul-source commented Jun 6, 2022 •

edited by jniesz

Loading

dthaler left a comment •

edited

Loading

jniesz Jun 14, 2022 •

edited

Loading