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

✨ Use crane to add hash suggestion to unpinned Docker images #2037

Merged
merged 7 commits into from
Jul 19, 2022

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

This PR attempts to address an enhancement in #967

What is the current behavior?

A warning is generated when a Docker dependency is not pinned.

Warn: containerImage not pinned by hash: Dockerfile:1

What is the new behavior (if this is a feature change)?**

A hash is now provided, as determined by crane

Warn: containerImage not pinned by hash. Fix by updating golang:1.17 to golang:1.17@sha256:e4d220556e80a686fad8874b40ccfa2f3172dfe5c6ca7f36f8ee79d594a9c959: Dockerfile:1

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #967

Special notes for your reviewer

NONE

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Hash suggestions are now made for unpinned images in Dockerfiles

@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #2037 (d2df929) into main (a905d66) will increase coverage by 2.63%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##             main    #2037      +/-   ##
==========================================
+ Coverage   42.03%   44.66%   +2.63%     
==========================================
  Files          92       92              
  Lines        7554     7563       +9     
==========================================
+ Hits         3175     3378     +203     
+ Misses       4130     3930     -200     
- Partials      249      255       +6     

@spencerschrock spencerschrock force-pushed the docker-suggest-pin-hash branch from a9ea16d to 1c99e0a Compare July 13, 2022 20:35
@spencerschrock spencerschrock temporarily deployed to integration-test July 13, 2022 20:35 Inactive
@github-actions
Copy link

@naveensrinivasan
Copy link
Member

@spencerschrock Thanks!

Is this PR going to provide only SHA for Linux/amd64?

Crane provides an option to https://github.com/google/go-containerregistry/blob/d187a716b559771a3768caa9c4dba796cb4650b3/pkg/v1/platform.go#L24 https://github.com/google/go-containerregistry/blob/d187a716b559771a3768caa9c4dba796cb4650b3/pkg/crane/digest.go#L20

type Platform struct {
	Architecture string   `json:"architecture"`
	OS           string   `json:"os"`
	OSVersion    string   `json:"os.version,omitempty"`
	OSFeatures   []string `json:"os.features,omitempty"`
	Variant      string   `json:"variant,omitempty"`
	Features     []string `json:"features,omitempty"`
}

We pass the OS and Architecture, which should provide us with other platforms if there are.

This will not limit us Linux/amd64. For example, community is moving to arm64 https://cloud.google.com/blog/products/containers-kubernetes/gke-supports-new-arm-based-tau-t2a-vms

On that note, we should have standard comment blocks for these remediations.

We would need some standard prefix for our recommendations so that customers can grep for it.

Thoughts?

@spencerschrock
Copy link
Member Author

I was aware of the crane options, but wasn't sure how to determine things on the scorecard side of things. From what I can tell, the same unpinned Dockerfile can be used on different platforms and work accordingly. We also can't make assumptions based on the platform where scorecard is running as it may differ from where the Dockerfile is deployed. So I was thinking we need some sort of option or metadata added. Is there anything already existing?

On that note, we should have standard comment blocks for these remediations.
We would need some standard prefix for our recommendations so that customers can grep for it.

+1
I think #1874 would cover this

@laurentsimon
Copy link
Contributor

On that note, we should have standard comment blocks for these remediations.
We would need some standard prefix for our recommendations so that customers can grep for it.

+1 I think #1874 would cover this

+1, the JSON should do that for free. /cc @raghavkaul

@spencerschrock spencerschrock force-pushed the docker-suggest-pin-hash branch from 1c99e0a to 8dedd4c Compare July 15, 2022 22:16
@spencerschrock spencerschrock marked this pull request as ready for review July 15, 2022 23:10
@spencerschrock
Copy link
Member Author

And to address @naveensrinivasan 's earlier point:

Is this PR going to provide only SHA for Linux/amd64?
Crane provides an option ...

crane's default should be good for multi-arch support. more detail in #967

@spencerschrock spencerschrock force-pushed the docker-suggest-pin-hash branch from 8dedd4c to e7848c4 Compare July 16, 2022 00:35
@spencerschrock spencerschrock temporarily deployed to integration-test July 16, 2022 00:35 Inactive
@github-actions
Copy link

Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

Thanks. Maybe add a few unit tests for the formatting part.

pkg/common.go Show resolved Hide resolved
@spencerschrock spencerschrock force-pushed the docker-suggest-pin-hash branch from e7848c4 to d2df929 Compare July 19, 2022 17:32
@spencerschrock spencerschrock temporarily deployed to integration-test July 19, 2022 17:32 Inactive
@github-actions
Copy link

@laurentsimon laurentsimon merged commit 096cbd0 into ossf:main Jul 19, 2022
@spencerschrock spencerschrock deleted the docker-suggest-pin-hash branch July 19, 2022 18:49
singhsaurabh pushed a commit to singhsaurabh/scorecard that referenced this pull request Jul 25, 2022
* Use crane to add hash suggestion to unpinned Docker images

* Add nil check before dereferencing name for image digest

* Reformat changes to comply with linter

* Add basic remediation for dockerfile pinning

* Deduplicate remediation code

* Remove reference to linux/amd64, as crane digest should be universal

* add remediation info to scorecard output. switch to using strings.Builder for more maintainable logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature - Provide docker image sha for failed pinned dependencies
3 participants