Skip to content

Added warning and test for lifecycle under which the builder is trusted#1553

Merged
dfreilich merged 2 commits intobuildpacks:mainfrom
cgoxo:1540-fix
Nov 20, 2022
Merged

Added warning and test for lifecycle under which the builder is trusted#1553
dfreilich merged 2 commits intobuildpacks:mainfrom
cgoxo:1540-fix

Conversation

@cgoxo
Copy link
Copy Markdown
Contributor

@cgoxo cgoxo commented Nov 14, 2022 •
edited
Loading

Summary

Added a warning for --lifecycle-image if the builder is trusted. Along with this the test case of this scenario was also added in ./interal/commands/build_test.go file

Output

A warning appears while building lifecycle image that since the builder is trusted, it would have the default lifecycle version

Before

After

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1540

@cgoxo cgoxo requested review from a team as code owners November 14, 2022 22:13
@github-actions github-actions Bot added this to the 0.28.0 milestone Nov 14, 2022
@github-actions github-actions Bot added the type/enhancement Issue that requests a new feature or improvement. label Nov 14, 2022
jjbustamante
jjbustamante requested changes Nov 15, 2022
View reviewed changes
Comment thread internal/commands/build_test.go
@github-actions github-actions Bot added type/chore Issue that requests non-user facing changes. and removed type/chore Issue that requests non-user facing changes. labels Nov 15, 2022
@cgoxo
Copy link
Copy Markdown
Contributor Author

cgoxo commented Nov 15, 2022 •
edited
Loading

Hello @natalieparellano, @jjbustamante! I wanted to specify the version called from the lifecycle image in logger.Warn(), but this takes only one argument.
I also tried to fix the issue of signed-off to pass DCO checks but it ended up showing your previous commits in my branch.

@cgoxo cgoxo force-pushed the 1540-fix branch 4 times, most recently from 2ef80eb to 825395b Compare November 16, 2022 22:48
@github-actions github-actions Bot added the type/chore Issue that requests non-user facing changes. label Nov 16, 2022
@github-actions github-actions Bot removed the type/chore Issue that requests non-user facing changes. label Nov 16, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 16, 2022 •
edited
Loading

Codecov Report

Merging #1553 (7081aa8) into main (558ae9a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1553      +/-   ##
==========================================
+ Coverage   80.99%   80.99%   +0.01%     
==========================================
  Files         156      156              
  Lines       10281    10284       +3     
==========================================
+ Hits         8326     8329       +3     
  Misses       1458     1458              
  Partials      497      497
Flag Coverage Δ
os_linux 79.76% <100.00%> (-0.04%) ⬇️
os_macos 77.28% <100.00%> (?)
os_windows 80.87% <100.00%> (+0.01%) ⬆️
unit 80.99% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@cgoxo cgoxo force-pushed the 1540-fix branch 2 times, most recently from c8eb697 to 09984fd Compare November 17, 2022 00:43
@cgoxo cgoxo changed the title Added test for lifecycle under builder is trusted Added warning and test for lifecycle under which the builder is trusted Nov 17, 2022
@cgoxo
Added test for lifecycle under builder is trusted
716b3ff 
Signed-off-by: cgoxo <goelchitresh13@gmail.com>

added warning statement at build_test.go file

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

Implemented warning of lifecycle image under trusted builder

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

Modified test for lifecycle-image is provided

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

added command.Execute() to test for builder is trusted and lifecycle image is provided

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

code reduction by adding it.Before()

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

added cfg and command under it.Before()

Signed-off-by: cgoxo <goelchitresh13@gmail.com>

fix linting errors

Signed-off-by: cgoxo <goelchitresh13@gmail.com>
jjbustamante
jjbustamante approved these changes Nov 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

Great Job @cgoxo!

Thanks for you PR ❤️

@cgoxo
Copy link
Copy Markdown
Contributor Author

cgoxo commented Nov 17, 2022 •
edited
Loading

Thank you @jjbustamante for your guidance. I won't be able to solve this issue without your help.

natalieparellano
natalieparellano reviewed Nov 17, 2022
View reviewed changes
Comment thread internal/commands/build.go Outdated
natalieparellano
natalieparellano reviewed Nov 17, 2022
View reviewed changes
Comment thread internal/commands/build_test.go Outdated
@natalieparellano
Copy link
Copy Markdown
Member

natalieparellano commented Nov 17, 2022

Awesome work @cgoxo! Thanks for the PR!

@cgoxo
Added suggestions in log message
7081aa8 
Signed-off-by: cgoxo <goelchitresh13@gmail.com>
@cgoxo
Copy link
Copy Markdown
Contributor Author

cgoxo commented Nov 17, 2022

Awesome work @cgoxo! Thanks for the PR!

Thank you so much @natalieparellano for guiding me on how to proceed.

@cgoxo
Copy link
Copy Markdown
Contributor Author

cgoxo commented Nov 17, 2022

Thank you @natalieparellano for approving the changes. May I know when the issue will get merged?

@jjbustamante
Copy link
Copy Markdown
Member

jjbustamante commented Nov 18, 2022

@dfreilich Could you take a look at this PR from a new contributor?

Also, @samj1912 or @jkutner

dfreilich
dfreilich approved these changes Nov 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks for the help, @cgoxo!

@dfreilich dfreilich merged commit 007306f into buildpacks:main Nov 20, 2022
@cgoxo
Copy link
Copy Markdown
Contributor Author

cgoxo commented Nov 20, 2022

Thank You @dfreilich, @jjbustamante, @natalieparellano! This surely gives me the confidence to contribute more to open-source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjbustamante jjbustamante jjbustamante approved these changes

@natalieparellano natalieparellano natalieparellano approved these changes

+1 more reviewer

@dfreilich dfreilich dfreilich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/enhancement Issue that requests a new feature or improvement.

Projects

None yet

Milestone

0.28.0

Development

Successfully merging this pull request may close these issues.

Pack build silently ignores --lifecycle-image if the builder is trusted

4 participants

@cgoxo @natalieparellano @jjbustamante @dfreilich