Registry Auth #43
Merged
Registry Auth #43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ekcasey
force-pushed
the
registry-auth
branch
6 times, most recently
from
c16d7b7
to
127108a
Compare
|
This PR improves performance of lifecycle when writing to and writing from a docker daemon. Exporter and Analyzer can now always run inside a container. We removed flags that supported other workflows (used by pack). |
sclevine
reviewed
Dec 6, 2018
analyzer.go
Outdated
| func (a *Analyzer) Analyze(launchDir string, config AppImageMetadata) error { | ||
| func (a *Analyzer) Analyze(image image.Image, launchDir string) error { | ||
| found, err := image.Found() | ||
|
|
analyzer.go
Outdated
| return err | ||
| } | ||
|
|
||
| if found { |
There was a problem hiding this comment.
Use structure:
if !found {
// unhappy, skip analyze
}
// happy, get metadata
Sort of like: https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow
analyzer.go
Outdated
| return "", cmd.FailErr(err, "image configfile", repoName) | ||
| func (a *Analyzer) getMetadata(image image.Image) (*AppImageMetadata, error) { | ||
| label, err := image.Label(MetadataLabel) | ||
| if err != nil && strings.Contains(err.Error(), "does not exist") { |
There was a problem hiding this comment.
Is there a sentinel error we can match on here? If not, this is fine.
analyzer_test.go
Outdated
| it.Before(func() { | ||
| image.EXPECT().Found().Return(true, nil) | ||
| image.EXPECT().Label("io.buildpacks.lifecycle.metadata").Return(` | ||
| {"buildpacks": [ |
analyzer_test.go
Outdated
| } | ||
| ]}`, nil) | ||
| }) | ||
| // |
cmd/analyzer/main.go
Outdated
| if metadataPath != "" { | ||
| bMetadata, err := ioutil.ReadFile(metadataPath) | ||
| var err error | ||
| var imageNotAPackageYo image.Image |
There was a problem hiding this comment.
oops lol, we meant to change that
* This allows the platform to provide authorization to analyzer/exporter running in a container without the proper credentials Signed-off-by: Emily Casey <ecasey@pivotal.io>
* Daemon implementation is more performant than go-containerregistry for our use case * Moved from pack Signed-off-by: Jacques Chester <jchester@pivotal.io> Signed-off-by: Emily Casey <ecasey@pivotal.io>
Signed-off-by: Emily Casey <ecasey@pivotal.io> Signed-off-by: Jacques Chester <jchester@pivotal.io>
Signed-off-by: Matthew McNew <mmcnew@pivotal.io> Signed-off-by: Andrew Meyer <ameyer@pivotal.io>
Signed-off-by: Matthew McNew <mmcnew@pivotal.io> Signed-off-by: Emily Casey <ecasey@pivotal.io>
Signed-off-by: Matthew McNew <mmcnew@pivotal.io> Signed-off-by: Emily Casey <ecasey@pivotal.io>
* analyzer and exporter can be run on build or builder image Signed-off-by: Emily Casey <ecasey@pivotal.io> Signed-off-by: Matthew McNew <mmcnew@pivotal.io>
Signed-off-by: Emily Casey <ecasey@pivotal.io> Signed-off-by: Matthew McNew <mmcnew@pivotal.io>
ekcasey
force-pushed
the
registry-auth
branch
from
0d5d18f
to
59ac786
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
introduces
PACK_REGISTRY_AUTHenv var which allow platform to provide lifecycle with registry authorization