Skip to content

Authentication config file for in-memory backend#59

Merged
LaurenSpiegel merged 7 commits intomasterfrom
dev/ft/FileAuth
Jul 1, 2016
Merged

Authentication config file for in-memory backend#59
LaurenSpiegel merged 7 commits intomasterfrom
dev/ft/FileAuth

Conversation

@alexandre-merle
Copy link
Copy Markdown
Contributor

"This PR provides a new config file format for the memory backend's authentication data, as well as a number of small utilities.

Edit: Please note that the checker writes the errors into the standard error output stream, in order for the user to see them. I am unsure whether this is the best solution or not, but I wanted to do that to ensure early failure if the configuration wasn't right. The thrown error in this case might deserve improvement also. I'm open to suggestions on that side :)"

Reopening of #8

@alexandre-merle
Copy link
Copy Markdown
Contributor Author

@ironman-machine give me your status about that :)

@ironman-machine
Copy link
Copy Markdown
Contributor

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"DEFAULT_BRANCH":"master","SCALITY_S3_BRANCH":"dev/ft/FileAuth"}

Please follow http://ci.ironmann.io/gh/scality/Integration/1779 for circle status.

Comment thread lib/auth/in_memory/checker.js Outdated
}
checkExists(data, 'User', 'keys', 'array', userObj);

if (userObj.keys) {
Copy link
Copy Markdown
Contributor

@MathieuCassagne MathieuCassagne Jun 17, 2016

Choose a reason for hiding this comment

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

Can't you do just like:

if (checkExists(data, 'User', 'keys', 'array', userObj))
    userObj.keys.forEach(keyObj => {
        // eslint-disable-next-line no-param-reassign
        data.keys[keyObj.access] = incr(data.keys[keyObj.access]);
    });
}

@alexandre-merle alexandre-merle force-pushed the dev/ft/FileAuth branch 2 times, most recently from a992126 to 9b9ff8e Compare June 17, 2016 14:02
@alexandre-merle
Copy link
Copy Markdown
Contributor Author

@ironman-machine give me your status about that :)

@ironman-machine
Copy link
Copy Markdown
Contributor

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"DEFAULT_BRANCH":"master","SCALITY_S3_BRANCH":"dev/ft/FileAuth"}

Please follow http://ci.ironmann.io/gh/scality/Integration/1780 for circle status.

if (nerr > 0) {
return true;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I'm reading this correctly, we log errors for duplicate items and then don't log any other formatting errors if there are duplicates? Why? Also, a few more comments would be helpful here.

@ghost
Copy link
Copy Markdown

ghost commented Jun 21, 2016

Don't we want this in Arsenal instead?

checkData.errors.push({
txt: 'no "accounts" array defined in Auth config',
});
return dumpErrors(checkData);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't understand, why check here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, I'm not sure I get the question, but my go-to answer is: There are no accounts to browse anyways, might as well return early.

@alexandre-merle
Copy link
Copy Markdown
Contributor Author

@ironman-machine and again :)

@ironman-machine
Copy link
Copy Markdown
Contributor

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"DEFAULT_BRANCH":"master","SCALITY_S3_BRANCH":"dev/ft/FileAuth"}

Please follow http://ci.ironmann.io/gh/scality/Integration/1878 for circle status.

@ghost
Copy link
Copy Markdown

ghost commented Jun 30, 2016

End-to-End tests passed on https://github.com/scality/Integration/pull/74 both before this PR (http://ci.ironmann.io/gh/scality/Integration/1881) and after (http://ci.ironmann.io/gh/scality/Integration/1882)

Please review this so that we can FINALLY merge it...

@alexandre-merle
Copy link
Copy Markdown
Contributor Author

Agreed with @DavidPineauScality , this pull request is open since 1 month now :)

@LaurenSpiegel
Copy link
Copy Markdown
Contributor

There are open comments...

David Pineau added 2 commits June 30, 2016 21:07
 - Use new auth config file in Config.js
 - Define a new env variable to change the path of the authentication config
 file
 - Update backend.js to use the new authentication data API
 - Update tests to remove references to the old, inconsistent auth config file
 format.
@ghost ghost force-pushed the dev/ft/FileAuth branch from 312fd9a to c4b70e9 Compare June 30, 2016 19:07
@ghost
Copy link
Copy Markdown

ghost commented Jun 30, 2016

@LaurenSpiegel @rahulreddy updated :)

@ghost
Copy link
Copy Markdown

ghost commented Jun 30, 2016

Btw, https://github.com/scality/Integration/pull/74 need to be merged (and pushed to wip/ultron) first :)

@rahulreddy
Copy link
Copy Markdown
Contributor

👍 Thanks @DavidPineauScality and @alexandre-merle !

@LaurenSpiegel
Copy link
Copy Markdown
Contributor

@ironman-machine just to be sure

@ironman-machine
Copy link
Copy Markdown
Contributor

Hello @LaurenSpiegel,

Starting end to end procedure using the following payload:
{"DEFAULT_BRANCH":"master","SCALITY_S3_BRANCH":"dev/ft/FileAuth"}

Please follow http://ci.ironmann.io/gh/scality/Integration/1937 for circle status.

@LaurenSpiegel
Copy link
Copy Markdown
Contributor

End to end passed. Merging.

@LaurenSpiegel LaurenSpiegel merged commit e43b950 into master Jul 1, 2016
@ghost ghost deleted the dev/ft/FileAuth branch July 4, 2016 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants