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

Add struct and functionality for using user input #1387

Merged
merged 2 commits into from
Dec 21, 2022

Conversation

rbehjati
Copy link
Contributor

@rbehjati rbehjati commented Dec 15, 2022

Ref #1191

Builds on top of #1386 and adds struct and functionality for parsing and using user-provided input.

Moves InputOptions to package pkg to be able to pass it as an argument to NewDockerBuildConfig without introducing a circular dependency.

internal/builders/docker/commands.go Outdated Show resolved Hide resolved
internal/builders/docker/pkg/config_test.go Outdated Show resolved Hide resolved
internal/builders/docker/testdata/config.toml Outdated Show resolved Hide resolved
internal/builders/docker/pkg/common.go Outdated Show resolved Hide resolved
internal/builders/docker/pkg/common.go Outdated Show resolved Hide resolved
internal/builders/docker/pkg/config.go Show resolved Hide resolved
internal/builders/docker/pkg/config.go Show resolved Hide resolved
@rbehjati rbehjati force-pushed the builder-3 branch 2 times, most recently from 8029375 to 088ccd9 Compare December 20, 2022 22:52
Copy link
Contributor Author

@rbehjati rbehjati left a comment

Choose a reason for hiding this comment

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

Thanks for the review.

internal/builders/docker/pkg/config_test.go Outdated Show resolved Hide resolved
internal/builders/docker/testdata/config.toml Outdated Show resolved Hide resolved
internal/builders/docker/pkg/config.go Show resolved Hide resolved
internal/builders/docker/pkg/config.go Show resolved Hide resolved
@rbehjati rbehjati force-pushed the builder-3 branch 4 times, most recently from 9db3d71 to 6ab04cb Compare December 20, 2022 23:22

// NewDockerBuildConfig validates the inputs and generates an instance of
// DockerBuildConfig.
func NewDockerBuildConfig(io *InputOptions) (*DockerBuildConfig, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could make the fields in DockerBuildConfig private/unexposed to make sure it is not possible to create an instance of it without performing these checks. What do you think? @asraa & @ianlewis (somewhat related to the comment below on validating the path)

@rbehjati
Copy link
Contributor Author

@asraa @laurentsimon @ianlewis I am happy with this PR. Can you please merge it?

@laurentsimon
Copy link
Collaborator

Can you rebase and solve the conflicts?

@laurentsimon laurentsimon enabled auto-merge (squash) December 21, 2022 17:16
Signed-off-by: Razieh Behjati <razieh@google.com>
Signed-off-by: Razieh Behjati <razieh@google.com>
auto-merge was automatically disabled December 21, 2022 17:23

Head branch was pushed to by a user without write access

@rbehjati
Copy link
Contributor Author

Can you rebase and solve the conflicts?

Done. But now the CI step for verify base image fails. I don't think that is due to my changes. Is it?

@laurentsimon
Copy link
Collaborator

No it's not your code. @ianlewis thoughts on what's going on?
I'm not sure there is a commitment from the image maintainers to sign the images. I'll merge this PR to unblock you.
Filed #1401 to resolve later.

@laurentsimon laurentsimon merged commit ac2cab7 into slsa-framework:main Dec 21, 2022
@rbehjati
Copy link
Contributor Author

Many thanks :)

@rbehjati rbehjati deleted the builder-3 branch December 21, 2022 17:36
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.

3 participants