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.

Sign up for GitHub

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

Jump to bottom

pkg/podman: Add error parsing method #786

Closed
wants to merge 1 commit into from
Closed

pkg/podman: Add error parsing method #786

wants to merge 1 commit into from

Conversation

HarryMichal
Copy link
Member

This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

The structure is only meant to be used internally.

@HarryMichal HarryMichal added 3. Enhancement Improvement to an existing feature 6. Minor Change Should not cause breakage 2. Under The Hood Multiple areas of the app are influenced by this ticket labels Jun 7, 2021
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 7, 2021
@HarryMichal
pkg/podman: Add error parsing method
8bee17e 
This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

The structure is only meant to be used internally.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from afd427d to 8bee17e Compare June 7, 2021 19:55
@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal HarryMichal mentioned this pull request Jun 7, 2021
Closed
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 12, 2021
@HarryMichal
pkg/podman: Add error parsing method
f7773d9 
This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

The structure is only meant to be used internally.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 8bee17e to f7773d9 Compare June 12, 2021 16:58
@softwarefactory-project-zuul
Copy link

Build succeeded.

HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 21, 2021
@HarryMichal
pkg/podman: Add error parsing method
0f5868c 
This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

The structure is only meant to be used internally.

containers#786
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 21, 2021
@HarryMichal
pkg/podman: Add error parsing method
e81af81 
This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from f7773d9 to e81af81 Compare June 21, 2021 21:48
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 21, 2021
@HarryMichal
pkg/podman: Add error parsing method
38d9ed6 
This new helper function parses the output of Podman in stderr into
a dedicated structure with defined methods for working with the error.

The most significant part is the method Is() that wraps around
errors.Is() and if that fails to find a match resorts to string
matching. This is needed because there are no defined errors to catch
errors coming from Podman.

containers#786
@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal HarryMichal added this to the Release 0.1.0 milestone Jun 25, 2021
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 29, 2021
@HarryMichal
pkg/podman: Add error parsing method
66175db 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from e81af81 to 66175db Compare June 29, 2021 16:08
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 29, 2021
@HarryMichal
pkg/podman: Add error parsing method
99fc1b7 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 66175db to 99fc1b7 Compare June 29, 2021 16:12
@softwarefactory-project-zuul
Copy link

Build failed.

HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 1, 2021
@HarryMichal
pkg/podman: Add error parsing method
2c4c3bc 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 99fc1b7 to 2c4c3bc Compare July 1, 2021 10:03
@softwarefactory-project-zuul
Copy link

Build failed.

debarshiray
debarshiray reviewed Jul 1, 2021
View reviewed changes
Copy link
Member

@debarshiray debarshiray 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 updating the branch, @HarryMichal ! This looks really good. One small question:

src/pkg/podman/podman.go Outdated Show resolved Hide resolved
debarshiray
debarshiray reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

We should add some unit tests for the internalError implementation.

src/pkg/podman/podman.go Outdated Show resolved Hide resolved
@HarryMichal
Copy link
Member Author

We should add some unit tests for the internalError implementation.

Yes, we should. The setup will be a bit tricky since the type is not exported but Go should have a way to go around the fact.

HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
15e4257 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 2c4c3bc to 15e4257 Compare July 3, 2021 12:21
@softwarefactory-project-zuul
Copy link

Build failed.

HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
d5ae48d 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
cf70190 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 15e4257 to cf70190 Compare July 3, 2021 15:26
@softwarefactory-project-zuul
Copy link

Build failed.

debarshiray
debarshiray reviewed Jul 3, 2021
View reviewed changes
src/pkg/podman/error.go Outdated Show resolved Hide resolved
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
328a9ab 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from cf70190 to 328a9ab Compare July 3, 2021 23:29
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
4fb2ae6 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal
pkg/podman: Add error parsing method
f130844 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal force-pushed the pkg/podman/error-parsing branch from 328a9ab to f130844 Compare July 3, 2021 23:38
HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jul 3, 2021
@HarryMichal
pkg/podman: Add error parsing method
a0246fc 
This new helper type serves for parsing the output of Podman in stderr
into a form that can be further analyzed.

The most significant part is the method Is() that lexically compares
error strings. This is needed because there are no defined errors to
catch errors coming from Podman.

containers#786
@HarryMichal HarryMichal added 3. New Feature New feature and removed 3. Enhancement Improvement to an existing feature labels Jul 3, 2021
@softwarefactory-project-zuul
Copy link

Build failed.

@debarshiray
Copy link
Member

Let's fold this pull request into #787

Otherwise it's becoming impossible to manage all the branches that include each other and are evolving in different ways.

@debarshiray debarshiray closed this Jul 4, 2021
@HarryMichal HarryMichal deleted the pkg/podman/error-parsing branch July 4, 2021 17:49
debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Jul 4, 2021
@HarryMichal @debarshiray
pkg/podman: Support parsing 'podman pull' spew into a Go error
62a0224 
This is meant to get a better understanding of a failed 'podman pull'
invocation to understand whether pulling an image requires logging into
the registry or not. Currently, 'podman pull' doesn't have a dedicated
exit code to denote authorization errors, so this is meant to be a
temporary workaround for that.

Parsing the error stream is inherently fragile and tricky because
there's no guarantee that the structure of the messages won't change,
and there's no clear definition of how the messages are laid out.
Therefore, this approach can't be treated as a generic solution for
getting detailed information about failed Podman invocations.

The error stream is used not only for dumping error messages, but also
for showing progress bars. Therefore, all lines are skipped until one
that starts with "Error: " is found. This is a heuristic based on how
Go programs written using the Cobra [1] library tend to report errors.

All subsequent lines are taken together and split around the ": "
sub-string, on the assumption that the ": " sub-string is used when a
new error message is prefixed to an inner error. Each sub-string
created from the split is treated as a potential member of the chain of
errors reported within Podman.

Some real world examples of the 'podman pull' error stream in the case
of authorization errors are:

  * With Docker Hub (https://hub.docker.com/):
    Trying to pull docker.io/library/foobar:latest...
    Error: Error initializing source docker://foobar:latest: Error reading manifest latest in docker.io/library/foobar: errors:
    denied: requested access to the resource is denied
    unauthorized: authentication required

  * With registry.redhat.io:
    Trying to pull registry.redhat.io/foobar:latest...
    Error: Error initializing source docker://registry.redhat.io/foobar:latest: unable to retrieve auth token: invalid username/password: unauthorized: Please login to the Red Hat Registry using your Customer Portal credentials. Further instructions can be found here: https://access.redhat.com/RegistryAuthentication

[1] https://github.com/spf13/cobra/
    https://pkg.go.dev/github.com/spf13/cobra

containers#786
containers#787
debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Jul 4, 2021
@HarryMichal @debarshiray
pkg/podman: Support parsing 'podman pull' spew into a Go error
b93cd72 
This is meant to get a better understanding of a failed 'podman pull'
invocation to understand whether pulling an image requires logging into
the registry or not. Currently, 'podman pull' doesn't have a dedicated
exit code to denote authorization errors, so this is meant to be a
temporary workaround for that.

Parsing the error stream is inherently fragile and tricky because
there's no guarantee that the structure of the messages won't change,
and there's no clear definition of how the messages are laid out.
Therefore, this approach can't be treated as a generic solution for
getting detailed information about failed Podman invocations.

The error stream is used not only for dumping error messages, but also
for showing progress bars. Therefore, all lines are skipped until one
that starts with "Error: " is found. This is a heuristic based on how
Go programs written using the Cobra [1] library tend to report errors.

All subsequent lines are taken together and split around the ": "
sub-string, on the assumption that the ": " sub-string is used when a
new error message is prefixed to an inner error. Each sub-string
created from the split is treated as a potential member of the chain of
errors reported within Podman.

Some real world examples of the 'podman pull' error stream in the case
of authorization errors are:

  * With Docker Hub (https://hub.docker.com/):
    Trying to pull docker.io/library/foobar:latest...
    Error: Error initializing source docker://foobar:latest: Error reading manifest latest in docker.io/library/foobar: errors:
    denied: requested access to the resource is denied
    unauthorized: authentication required

  * With registry.redhat.io:
    Trying to pull registry.redhat.io/foobar:latest...
    Error: Error initializing source docker://registry.redhat.io/foobar:latest: unable to retrieve auth token: invalid username/password: unauthorized: Please login to the Red Hat Registry using your Customer Portal credentials. Further instructions can be found here: https://access.redhat.com/RegistryAuthentication

[1] https://github.com/spf13/cobra/
    https://pkg.go.dev/github.com/spf13/cobra

containers#689
containers#786
containers#787
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@debarshiray debarshiray debarshiray left review comments

Assignees
No one assigned
Labels
2. Under The Hood Multiple areas of the app are influenced by this ticket 3. New Feature New feature 6. Minor Change Should not cause breakage
Projects
None yet
Milestone
Release 0.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@HarryMichal @debarshiray