Create a bazel rule to print Envoy's dependencies by dprotaso · Pull Request #5049 · envoyproxy/envoy

dprotaso · 2018-11-15T14:57:26Z

Description:

Simply invoke the run rule as follows:

bazel run //bazel:print_dependencies

Or the python script directly (from project root):

python bazel/print_dependencies.py

Risk Level:
Low

Testing:
I tested the changes by invoking the script ci/build_container/docker_build.sh

Docs Changes:
None

Release Notes:
None.

Additional Notes
For build recipes there was a debate in slack about whether to fetch the GitHub autogenerated source tarballs or if possible one's provided by the maintainer in a release. Based on the opinions I opt'd the leave everything the same.

If we were to always fetch via autogenerated source tarballs using the git sha this would simplify the build scripts. I don't have preference just an FYI for others.

This was my original approach in an older branch that you can see here:
Versions file:
https://github.com/dprotaso/envoy/blob/9474cf3290020464e4fe31df7fcf236cd155d630/ci/build_container/build_recipes/versions.sh

Example script:
https://github.com/dprotaso/envoy/blob/9474cf3290020464e4fe31df7fcf236cd155d630/ci/build_container/build_recipes/benchmark.sh

lizan · 2018-11-15T15:38:42Z

Trying to under the context more:
What is the use case of the script? (What are we try to do with the output)
Can you provide a sample output?

dprotaso · 2018-11-15T16:51:39Z

Sample output is here:
https://gist.github.com/dprotaso/e0ce9fb104bbdbb75a577fc86575583d

What is the use case of the script? (What are we try to do with the output)

I'm trying to help facilitate OSS license scanning of Envoy & it's dependencies. The script produces an artifact that should be consumable by an automated process.

dprotaso · 2018-11-15T17:20:55Z

For example the Istio project currently scans it's go dependencies and discloses it here
https://raw.githubusercontent.com/istio/istio/1.1.0-snapshot.2/LICENSES.txt

This PR will enable similar scanning and disclosure for Envoy and its dependencies.

dprotaso · 2018-11-15T17:57:06Z

Looks like I overlooked
https://github.com/envoyproxy/envoy/blob/master/api/bazel/repositories.bzl

Is there a reason why this isn't in the root bazel folder?

moderation · 2018-11-15T18:35:59Z

From Slack yesterday, the 4 locations for dependencies are

api/bazel/repositories.bzl
bazel/repository_locations.bzl
ci/build_container/build_recipes/*
ci/build_container/build_container_common.sh

htuch · 2018-11-15T19:55:18Z

Highly recommend folks who haven't to take a look at #bazel on Slack from yesterday that @moderation points to, this is where we went over the context.

htuch

Neat

bazel/print_dependencies.py

dprotaso · 2018-11-15T23:36:53Z

@moderation @htuch I've made updates - do you mind taking a look?

Style should be consistent with other python files in the project
api/bazel/repositories.bzl version info is now in bazel/repository_locations.bzl
- I didn't want to muck with the existing logic in bazel/repository.bzl
- I opt'd to exposed the helper for api/bazel/repositories.bzl to declare repositories.

I also opt'd not to touch ci/build_container/build_container_common.sh since it's just installing buildifier.

lizan · 2018-11-16T02:20:43Z

I'm trying to help facilitate OSS license scanning of Envoy & it's dependencies. The script produces an artifact that should be consumable by an automated process.

If this is the purpose, why do you need git repo / SHA for every dependency? The download tar ball should have LICENSE{.txt} file in it, so you can inspect from there. The downloaded tar ball is where we consume dependency so it should be single source of truth.

Also take a look at the result of
bazel query 'kind(http_archive, //external:all)' --output=build (other output format is also available), it should have almost everything you need except recipes from ci/build_container/build_recipes/*. This way it will work for repos consuming Envoy.

api/bazel/repositories.bzl

dprotaso · 2018-11-16T03:30:53Z

it should have almost everything you need except recipes from ci/build_container/build_recipes/*

The main intent here is to capture all dependencies including the build recipes

If this is the purpose, why do you need git repo / SHA for every dependency? The download tar ball should have LICENSE{.txt} file in it, so you can inspect from there. The downloaded tar ball is where we consume dependency so it should be single source of truth.

Yeah ideally, we'd scan the source tarball. Some thoughts on why the repo/sha might still be relevant.

Traceability - not all dependencies are coming from a git repository. ie. python packages from PyPi. Tracing it back to the original repo may be important for some.
We can always fall back onto scanning the git repo/sha if scanning a manually created release tarball has any omissions. For example one of the twitter python packages has generated thrift code that includes no copyright notices.
Figured I should be consistent if I'm adding it for a few of the dependencies.

lizan · 2018-11-16T05:13:32Z

The main intent here is to capture all dependencies including the build recipes

I meant your script can utilize the output, without scraping everything else than ci/build_container/build_recipes/*. It will make the script more reusable in consuming repos (e.g. istio/proxy, or any other in-house repos. It also include those implicitly imported dependencies.

api/bazel/BUILD

api/bazel/repositories.bzl

htuch

Looks really nice now, I think this looks right structurally.

api/bazel/BUILD

api/bazel/repository_locations.bzl

htuch · 2018-11-25T23:36:53Z

api/bazel/repositories.bzl

-        strip_prefix = "protobuf-" + GOGOPROTO_RELEASE,
-        url = "https://github.com/gogo/protobuf/archive/v" + GOGOPROTO_RELEASE + ".tar.gz",
-        build_file_content = """
+GOGOPROTO_BUILD_CONTENT = """


Should this be at the top next to the other build content defs? Or should they all be next to their consuming sites? Either is good, as long as we are consistent.

htuch · 2018-11-25T23:37:38Z

bazel/print_dependencies.py

+RECIPE_VERSIONS_FILE = 'ci/build_container/build_recipes/versions.sh'
+
+
+# parseRecipeDetails will extact a build_recipe version


Nit: s/extact/extract/

htuch · 2018-11-25T23:38:30Z

bazel/print_dependencies.py

+
+# expandVars will replace references to environment
+# variables in the given 'value' string
+def expandVars(value):


Nit: not sure it's worth providing a wrapper for this..

htuch · 2018-11-25T23:40:28Z

ci/build_container/build_recipes/versions.sh

+
+GPERFTOOLS_VERSION=2.7
+GPERFTOOLS_TAG=gperftools-$GPERFTOOLS_VERSION
+GPERFTOOLS_FILE_URL=https://github.com/gperftools/gperftools/releases/download/$GPERFTOOLS_TAG/$GPERFTOOLS_TAG.tar.gz


Nit: Google style requires X="$Y" when you are expanding out variables, so best to just do this everywhere in this file.

htuch · 2018-11-25T23:42:22Z

ci/build_container/build_recipes/versions.sh

+ZLIB_VERSION=1.2.11
+ZLIB_FILE_URL=https://github.com/madler/zlib/archive/v$ZLIB_VERSION.tar.gz
+ZLIB_FILE_SHA256=629380c90a77b964d896ed37163f5c3a34f6e6d897311f1df2a7016355c45eff
+ZLIB_FILE_PREFIX=zlib-$ZLIB_VERSION


I've probably already mentioned this, but you could just make this a .py with a giant dictionary and a CLI driver that takes a single arg, which is the dep name. Then in the scripts, you can do $(versions.py zlib) in the build recipes when you want the zlib URL. It's slightly cleaner as you don't need to scrape as you do in the print recipes script using regexes, you could directly import.

You did mention it - I'll make the change it'll clean things up

lizan · 2018-11-28T13:03:11Z

api/bazel/envoy_http_archive.bzl

+    loc_key = kwargs.pop("repository_key", name)
+    location = repository_locations[loc_key]
+
+    # Git tags are mutable. We want to depend on commit IDs instead. Give the


I think git repositories are already disabled so we can remove this.

@lizan where are they disabled?

stale · 2018-12-06T20:05:29Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

dprotaso · 2018-12-07T14:02:39Z

Going to pick this up again once #5218 is merged

htuch · 2018-12-09T16:14:22Z

@dprotaso Ack, going to hopefully work on #5218 later today.

stale · 2018-12-16T16:19:39Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

htuch · 2018-12-23T12:18:22Z

@dprotaso interested in your thoughts on #5405, which proposes to also provide the version info dynamically to management servers.

stale · 2018-12-30T16:50:09Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

stale · 2019-01-06T23:35:18Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

htuch · 2019-01-18T19:00:16Z

Reopened, while we wait for external cmake support.

htuch · 2019-01-30T22:13:01Z

@dprotaso #5218 has merged.

dprotaso · 2019-01-30T22:17:50Z

Wooooooo

…

On Wed, Jan 30, 2019 at 17:13 htuch ***@***.***> wrote: @dprotaso <https://github.com/dprotaso> #5218 <#5218> has merged. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5049 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIgquTsDwxxNGZqY8AoaBgIfdAYJQAks5vIhkAgaJpZM4Yf9h_> .

stale · 2019-02-07T06:16:48Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

stale · 2019-02-14T07:07:06Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

dprotaso force-pushed the master branch 2 times, most recently from 0066854 to b0e7c96 Compare November 15, 2018 15:07

mattklein123 assigned lizan, htuch and jmillikin-stripe Nov 15, 2018

htuch suggested changes Nov 15, 2018

View reviewed changes

bazel/print_dependencies.py Outdated Show resolved Hide resolved

bazel/print_dependencies.py Outdated Show resolved Hide resolved

dprotaso changed the title ~~Create a bazel rule to print Envoy's dependencies~~ [WIP] Create a bazel rule to print Envoy's dependencies Nov 15, 2018

dprotaso force-pushed the master branch 5 times, most recently from 74a25b2 to 6d651a4 Compare November 15, 2018 22:03

dprotaso changed the title ~~[WIP] Create a bazel rule to print Envoy's dependencies~~ Create a bazel rule to print Envoy's dependencies Nov 15, 2018

lizan suggested changes Nov 16, 2018

View reviewed changes

api/bazel/repositories.bzl Outdated Show resolved Hide resolved

dprotaso force-pushed the master branch 2 times, most recently from 61c1914 to 1eceacc Compare November 16, 2018 04:46

dprotaso changed the title ~~Create a bazel rule to print Envoy's dependencies~~ [WIP] Create a bazel rule to print Envoy's dependencies Nov 16, 2018

dprotaso force-pushed the master branch from 1eceacc to bb6e0cd Compare November 16, 2018 05:10

dprotaso force-pushed the master branch from bb6e0cd to 6d8ffbf Compare November 16, 2018 05:21

dprotaso commented Nov 16, 2018

View reviewed changes

api/bazel/BUILD Outdated Show resolved Hide resolved

api/bazel/repositories.bzl Outdated Show resolved Hide resolved

dprotaso force-pushed the master branch from e5631e9 to 6389872 Compare November 22, 2018 19:34

htuch suggested changes Nov 25, 2018

View reviewed changes

lizan reviewed Nov 28, 2018

View reviewed changes

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 6, 2018

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Dec 7, 2018

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 16, 2018

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Dec 23, 2018

htuch mentioned this pull request Dec 24, 2018

build: migrate BoringSSL FIPS to genrule_repository. #5404

Merged

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 30, 2018

stale bot closed this Jan 6, 2019

htuch added no stalebot Disables stalebot from closing an issue waiting and removed stale stalebot believes this issue/PR has not been touched recently labels Jan 18, 2019

htuch reopened this Jan 18, 2019

htuch removed the no stalebot Disables stalebot from closing an issue label Jan 30, 2019

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 7, 2019

stale bot closed this Feb 14, 2019

dprotaso mentioned this pull request Feb 25, 2019

Include a python script to print Envoy's dependencies #6049

Merged

		RECIPE_VERSIONS_FILE = 'ci/build_container/build_recipes/versions.sh'


		# parseRecipeDetails will extact a build_recipe version

Conversation

dprotaso commented Nov 15, 2018

Uh oh!

lizan commented Nov 15, 2018

Uh oh!

dprotaso commented Nov 15, 2018

Uh oh!

dprotaso commented Nov 15, 2018

Uh oh!

dprotaso commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moderation commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

htuch commented Nov 15, 2018

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dprotaso commented Nov 15, 2018

Uh oh!

lizan commented Nov 16, 2018

Uh oh!

Uh oh!

dprotaso commented Nov 16, 2018

Uh oh!

lizan commented Nov 16, 2018

Uh oh!

Uh oh!

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Dec 6, 2018

Uh oh!

dprotaso commented Dec 7, 2018

Uh oh!

htuch commented Dec 9, 2018

Uh oh!

stale bot commented Dec 16, 2018

Uh oh!

htuch commented Dec 23, 2018

Uh oh!

stale bot commented Dec 30, 2018

Uh oh!

stale bot commented Jan 6, 2019

Uh oh!

htuch commented Jan 18, 2019

Uh oh!

htuch commented Jan 30, 2019

Uh oh!

dprotaso commented Jan 30, 2019 via email

Uh oh!

stale bot commented Feb 7, 2019

Uh oh!

stale bot commented Feb 14, 2019

Uh oh!

Reviewers

Assignees

dprotaso commented Nov 15, 2018 •

edited

Loading

moderation commented Nov 15, 2018 •

edited

Loading