Bug 1918879: better surface dockerconfigjson errors causing image pull errors #200

gabemontero · 2021-01-08T15:27:32Z

As part of the triage captured at https://coreos.slack.com/archives/C01JLQ7MB2M for a recent CI incident where a corrupted pull secret caused image pull failures with builds, it became apparent that V(4) trace was minimally needed to more precisely capture the error.

The PR strives to make the error cause more available with default logging.

@stevekuznetsov @alvaroaleman FYI

/assign @adambkaplan

@openshift/openshift-team-build-api FYI

stevekuznetsov

WIll this error message leak token content?

alvaroaleman · 2021-01-08T15:58:22Z

pkg/build/builder/cmd/dockercfg/cfg.go

 	var cfgJSON credentialprovider.DockerConfigJSON
 	if err := json.Unmarshal(contents, &cfgJSON); err != nil {
-		log.V(4).Infof("while trying to parse blob %q: %v", contents, err)
-		return false
+		log.V(0).Infof("error trying to parse blob %q: %v", contents, err)


Yeah, yesterday it looked like it ends up printing part of the secret, I don't think we can print the error itself by default. What about just printing "Error Deserializing, set env var X to value Y to see details (warning: This might leak the content of the secret"?

yep I'll redo this part

and to be clear, it was the contents piece here ... if we considered the error log that occurred here, while trying to parse blob << the raw json pull secret >> ": illegal base64 data at input byte 1152

The contents piece is << the raw json pull secret >> and the err string is illegal base64 data at input byte 1152

I'll leave the err string

I've also double checked json.Unmarshal ... all the errors just note the byte array location like in the log above

gabemontero · 2021-01-08T16:21:00Z

@stevekuznetsov @alvaroaleman separate commit pushed to remove byte array contents from the error log

alvaroaleman · 2021-01-08T16:23:49Z

pkg/build/builder/cmd/dockercfg/cfg.go

 		}
 	}
-	return ""
+	var err error
+	if len(errList) > 0 {


Nit: The condition isn't needed, utilerrors.NewAggregate will return a nil error if the error list has zero entries or all of them are nil

ah right thanks ... update pushed in separate commit

alvaroaleman

/lgtm

gabemontero · 2021-01-08T16:38:21Z

thanks @stevekuznetsov @alvaroaleman review commits squashed awaiting look-see from @adambkaplan

stevekuznetsov · 2021-01-08T16:39:09Z

/lgtm

openshift-ci-robot · 2021-01-21T16:25:04Z

@gabemontero: This pull request references Bugzilla bug 1918879, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1918879: better surface dockerconfigjson errors causing image pull errors

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

gabemontero · 2021-01-21T16:26:13Z

/cherrypick release-4.6

openshift-cherrypick-robot · 2021-01-21T16:26:14Z

@gabemontero: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you.

In response to this:

/cherrypick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

adambkaplan

/approve

openshift-ci-robot · 2021-01-22T13:50:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, alvaroaleman, gabemontero, stevekuznetsov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adambkaplan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2021-01-22T15:59:33Z

@gabemontero: All pull requests linked via external trackers have merged:

openshift/builder#200

Bugzilla bug 1918879 has been moved to the MODIFIED state.

In response to this:

Bug 1918879: better surface dockerconfigjson errors causing image pull errors

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2021-01-22T15:59:51Z

@gabemontero: #200 failed to apply on top of branch "release-4.6":

Applying: better surface dockerconfigjson errors causing image pull errors
Using index info to reconstruct a base tree...
M	pkg/build/builder/daemonless.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/build/builder/daemonless.go
CONFLICT (content): Merge conflict in pkg/build/builder/daemonless.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 better surface dockerconfigjson errors causing image pull errors
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot assigned adambkaplan Jan 8, 2021

openshift-ci-robot requested review from adambkaplan and coreydaley January 8, 2021 15:28

stevekuznetsov reviewed Jan 8, 2021

View reviewed changes

alvaroaleman reviewed Jan 8, 2021

View reviewed changes

alvaroaleman approved these changes Jan 8, 2021

View reviewed changes

openshift-ci-robot assigned alvaroaleman Jan 8, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 8, 2021

better surface dockerconfigjson errors causing image pull errors

d8a64de

gabemontero force-pushed the fix-docker-cfg-err-log branch from cab1567 to d8a64de Compare January 8, 2021 16:37

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 8, 2021

openshift-ci-robot assigned stevekuznetsov Jan 8, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 8, 2021

gabemontero changed the title ~~better surface dockerconfigjson errors causing image pull errors~~ Bug 1918879: better surface dockerconfigjson errors causing image pull errors Jan 21, 2021

openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Jan 21, 2021

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 21, 2021

adambkaplan approved these changes Jan 22, 2021

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2021

openshift-merge-robot merged commit 1cc2d63 into openshift:master Jan 22, 2021

gabemontero deleted the fix-docker-cfg-err-log branch January 22, 2021 16:01

gabemontero mentioned this pull request Jan 22, 2021

[release-4.6] Bug 1919359: better surface dockerconfigjson errors causing image pull errors #210

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1918879: better surface dockerconfigjson errors causing image pull errors #200

Bug 1918879: better surface dockerconfigjson errors causing image pull errors #200

gabemontero commented Jan 8, 2021

stevekuznetsov left a comment

alvaroaleman Jan 8, 2021

gabemontero Jan 8, 2021

gabemontero Jan 8, 2021

gabemontero Jan 8, 2021

gabemontero commented Jan 8, 2021

alvaroaleman Jan 8, 2021

gabemontero Jan 8, 2021

alvaroaleman left a comment

gabemontero commented Jan 8, 2021

stevekuznetsov commented Jan 8, 2021

openshift-ci-robot commented Jan 21, 2021

gabemontero commented Jan 21, 2021

openshift-cherrypick-robot commented Jan 21, 2021

adambkaplan left a comment

openshift-ci-robot commented Jan 22, 2021

openshift-ci-robot commented Jan 22, 2021

openshift-cherrypick-robot commented Jan 22, 2021

Bug 1918879: better surface dockerconfigjson errors causing image pull errors #200

Bug 1918879: better surface dockerconfigjson errors causing image pull errors #200

Conversation

gabemontero commented Jan 8, 2021

stevekuznetsov left a comment

Choose a reason for hiding this comment

alvaroaleman Jan 8, 2021

Choose a reason for hiding this comment

gabemontero Jan 8, 2021

Choose a reason for hiding this comment

gabemontero Jan 8, 2021

Choose a reason for hiding this comment

gabemontero Jan 8, 2021

Choose a reason for hiding this comment

gabemontero commented Jan 8, 2021

alvaroaleman Jan 8, 2021

Choose a reason for hiding this comment

gabemontero Jan 8, 2021

Choose a reason for hiding this comment

alvaroaleman left a comment

Choose a reason for hiding this comment

gabemontero commented Jan 8, 2021

stevekuznetsov commented Jan 8, 2021

openshift-ci-robot commented Jan 21, 2021

gabemontero commented Jan 21, 2021

openshift-cherrypick-robot commented Jan 21, 2021

adambkaplan left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 22, 2021

openshift-ci-robot commented Jan 22, 2021

openshift-cherrypick-robot commented Jan 22, 2021