Check 'auth' field when displaying .dockercfg config#2392
Check 'auth' field when displaying .dockercfg config#2392openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Conversation
app/scripts/services/secrets.js
Outdated
| return secretsByType; | ||
| }; | ||
|
|
||
| var setDataParams = function(decodedSecretData, serverName, data) { |
There was a problem hiding this comment.
If this is specific to dockercfg, we should have that somewhere in the function name.
There was a problem hiding this comment.
This method is not intended to be specific for dockercfg or config.json. It's purpose is to assemble an object that will contain server address, username, password.
app/scripts/services/secrets.js
Outdated
| username: usernamePassword[0], | ||
| password: usernamePassword[1], | ||
| email: data.email | ||
| }; |
There was a problem hiding this comment.
I would let the caller set the value in the auths object and just let this function return the data. Then the function takes fewer arguments and doesn't need to know anything about the structure of the decodedSecretData object passed to it.
(untested)
var getDockercfgServerParams = function(serverData) {
var params = _.pick(serverData, ['email', 'username', 'password']);
if (serverData.auth) {
try {
// Decode Base64-encoded username:password string.
_.spread(window.atob(data.auth).split(":"), function(username, password) {
params.username = username;
params.password = password;
});
} catch(e) {}
}
return params;
};then below in the decodeDockercfg function
decodedSecretData.auths[serverName] = getDockercfgServerParams(data);https://lodash.com/docs/4.17.4#pick
https://lodash.com/docs/4.17.4#spread
app/scripts/services/secrets.js
Outdated
| var setDataParams = function(decodedSecretData, serverName, data) { | ||
| var usernamePassword = []; | ||
| if (data.auth) { | ||
| usernamePassword = window.atob(data.auth).split(":"); |
There was a problem hiding this comment.
We should catch exceptions to avoid a runtime error if it's not properly encoded.
https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/atob
|
@spadgett I've updated the PR based on your comments. Tested the change with single and multi server PTAL |
|
/kind bug @jhadvig You have a dist mismatch |
|
/lgtm |
|
Hm. /retest |
|
Automatic merge from submit-queue. |

While working on https://bugzilla.redhat.com/show_bug.cgi?id=1506998 I've noticed that we are not checking for
authfield in the old .dockercfg format, which looks like:The
registryaddresscould also containusernameandpasswordfields.@spadgett PTAL
Will add the dist after review