Skip to content

feat: add docker build detection#409

Merged
behnazh-w merged 5 commits into
oracle:stagingfrom
timyarkov:docker-support
Sep 8, 2023
Merged

feat: add docker build detection#409
behnazh-w merged 5 commits into
oracle:stagingfrom
timyarkov:docker-support

Conversation

@timyarkov

@timyarkov timyarkov commented Aug 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR adds support for the detection of Dockerfiles, so as to cover scenarios where Docker gets used as a build tool, e.g. when Macaron gets built using its Dockerfile.

  • Docker presence is detected by finding files either named Dockerfile or in the formats *.Dockerfile or Dockerfile.* to cover different naming conventions of dockerfiles, e.g. dev.Dockerfile or like Macaron's own Dockerfile.base and Dockerfile.final. This is defined in defaults.ini under [builder.docker]
  • The supported build command keyword is build, and supported deploy command keyword is push, defined in defaults.ini under [builder.docker]
  • For CI deploy commands the Github action docker/build-push-action@v4 is supported, defined in defaults.ini under [builder.docker.ci.deploy]

To do from here:

  • Implement dependency resolution for dockerfiles, as of now this is purely detection and does not include anything regarding SBOMs
  • Further testing; as of now it has a single test for detection of docker using https://github.com/timyarkov/docker_test, however it would be good to expand tests to cover scenarios like multiple build tools alongside docker, more naming variations, etc.

@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 13, 2023
timyarkov
timyarkov commented Aug 13, 2023
View reviewed changes
Comment thread go.mod
@timyarkov timyarkov marked this pull request as draft August 13, 2023 07:55
@tromai

tromai commented Aug 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

It looks like the error your are encountering is related to this issue. Please rebase on staging as we have decided to ignore the error raised by pip_audit in this PR for now.

@timyarkov timyarkov changed the title feat: docker support feat: add docker build detection Aug 14, 2023
@timyarkov timyarkov marked this pull request as ready for review August 14, 2023 02:23
@tromai

tromai commented Aug 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@timyarkov It would be good if we have a description of the feature's behaviors in the PR's description whenever a new feature is developed. It could really help us with reviewing the code changes.
For example: #67, #316 and #254 (minus the implementation section).
Note that it doesn't need a full detail explanation of the implementation but a short description of how the features work as the user's perspective:

  • any configuration needed
  • what sort of inputs does it accept
  • what is the problem the feature is going to solve

@timyarkov

timyarkov commented Aug 17, 2023

Copy link
Copy Markdown
Contributor Author

@timyarkov It would be good if we have a description of the feature's behaviors in the PR's description whenever a new feature is developed. It could really help us with reviewing the code changes. For example: #67, #316 and #254 (minus the implementation section). Note that it doesn't need a full detail explanation of the implementation but a short description of how the features work as the user's perspective:

  • any configuration needed
  • what sort of inputs does it accept
  • what is the problem the feature is going to solve

My bad, have updated it with more details, hopefully that is enough?

@tromai

tromai commented Aug 17, 2023

Copy link
Copy Markdown
Contributor

@timyarkov It would be good if we have a description of the feature's behaviors in the PR's description whenever a new feature is developed. It could really help us with reviewing the code changes. For example: #67, #316 and #254 (minus the implementation section). Note that it doesn't need a full detail explanation of the implementation but a short description of how the features work as the user's perspective:

  • any configuration needed
  • what sort of inputs does it accept
  • what is the problem the feature is going to solve

My bad, have updated it with more details, hopefully that is enough?

No worries. That's great. Thanks for that.

tromai
tromai reviewed Aug 17, 2023
View reviewed changes
Comment thread src/macaron/config/defaults.ini
tromai
tromai reviewed Aug 17, 2023
View reviewed changes
Comment thread src/macaron/slsa_analyzer/build_tool/docker.py Outdated
@tromai

tromai commented Aug 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Please rebase on staging. Hopefully this PR would prevent this issue from happening again.
Also, it would be great if you push fixed commit (happens after you mark a PR as ready for review) separately without squashing it into previous commits. This would also help me keeping track of the changes too. Thanks!

tromai
tromai reviewed Aug 20, 2023
View reviewed changes
Comment thread src/macaron/config/defaults.ini
behnazh-w
behnazh-w reviewed Aug 25, 2023
View reviewed changes

@behnazh-w behnazh-w left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add unit tests following the examples for build tools here.

Comment thread src/macaron/config/defaults.ini
@timyarkov

timyarkov commented Aug 27, 2023

Copy link
Copy Markdown
Contributor Author

Please add unit tests following the examples for build tools here.

Added in latest commit 33d4595

@timyarkov timyarkov force-pushed the docker-support branch 2 times, most recently from c5ab096 to 7664721 Compare August 28, 2023 03:30
behnazh-w
behnazh-w reviewed Aug 31, 2023
View reviewed changes
Comment thread src/macaron/config/defaults.ini Outdated
@timyarkov
feat: add docker build detection
09e9737 
Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
@timyarkov
test: docker build tool tests
d3e27eb 
Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
@timyarkov
fix: updated e2e docker test
4869f0e 
Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
@timyarkov
fix: test docstring fix
de67959 
Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
@timyarkov
fix: fix docker in defaults.ini
82ca3df 
Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
@behnazh-w behnazh-w merged commit d15fffb into oracle:staging Sep 8, 2023
art1f1c3R pushed a commit that referenced this pull request Nov 29, 2024
@timyarkov
feat: add docker build detection (#409)
358ab75 
This PR adds support for the detection of Dockerfiles, so as to cover scenarios where Docker gets used as a build tool.

* Docker presence is detected by finding files either named Dockerfile or in the formats *.Dockerfile or Dockerfile.* to cover different naming conventions of dockerfiles, e.g. dev.Dockerfile or like Macaron's own Dockerfile.base and Dockerfile.final. This is defined in defaults.ini under [builder.docker]

* The supported build command keyword is build, and supported deploy command keyword is push, defined in defaults.ini under [builder.docker]

*For CI deploy commands the GitHub action docker/build-push-action is supported, defined in defaults.ini under [builder.docker.ci.deploy]

Signed-off-by: Tim Yarkov <timdyarkov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@behnazh-w behnazh-w behnazh-w approved these changes

+1 more reviewer

@tromai tromai tromai approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Milestone

Add support for container-based builders

Development

Successfully merging this pull request may close these issues.

3 participants

@timyarkov @tromai @behnazh-w