Alon Bar-Lev has posted comments on this change. Change subject: aaa: Extensions tester tool ......................................................................
Patch Set 6: (7 comments) http://gerrit.ovirt.org/#/c/27814/6/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java: Line 13: import org.ovirt.engine.api.extensions.ExtMap; Line 14: import org.ovirt.engine.api.extensions.ExtUUID; Line 15: import org.ovirt.engine.api.extensions.aaa.Authn; Line 16: import org.ovirt.engine.api.extensions.aaa.Authz; Line 17: import org.ovirt.engine.core.aaa.SearchQueryParsingUtils; please detach from aaa, single function is not good enough reason. Line 18: import org.ovirt.engine.core.extensions.mgr.ExtensionProxy; Line 19: import org.ovirt.engine.core.extensions.mgr.ExtensionsManager; Line 20: Line 21: public class ExtensionsTool { Line 28: this.extensionsManager = extensionsManager; Line 29: try { Line 30: switch (args.getAction()) { Line 31: case ExtensionsToolArguments.ACTION_AUTH_SEQUENCE: Line 32: doAuthSequence(args); args can be member, and better split into constructor and run() to perform the actual logic Line 33: break; Line 34: Line 35: case ExtensionsToolArguments.ACTION_QUERY_BY_IDS: Line 36: doQueryByIds(args); Line 84: user Line 85: ).mput( Line 86: Authn.InvokeKeys.CREDENTIALS, Line 87: password Line 88: ) please align... it should be single indent... xxx = mput( x, y ).mput( x, y ) I am unsure why it that complex with IDE, but please keep style sane. Line 89: ); Line 90: ExtMap authRecord = processAuthnResult(outMap, "AUTHENTICATE_CREDENTIALS"); Line 91: Line 92: if (args.get(ExtensionsToolArguments.ARG_AUTHZ_NAME) != null) { Line 286: log.info(String.format("AuthRecord: Principal %s, ValidTo: %s", Line 287: extMap.get(Authn.AuthRecord.PRINCIPAL, ""), Line 288: extMap.get(Authn.AuthRecord.VALID_TO, ""))); Line 289: } Line 290: } please add debug mode to dump the entire in/out, or add debug mode to enable the extension trace log, anything that will enable greater visibility. Line 291: http://gerrit.ovirt.org/#/c/27814/6/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java: Line 30: // Auth arguments Line 31: public static final String ARG_USER = "--user"; Line 32: public static final String ARG_AUTHN_NAME = "--authn"; Line 33: public static final String ARG_PASSWORD_FILE = "--password-file"; Line 34: public static final String ARG_PASSWORD_ENV_KEY = "--password-env-key"; no... please do this exactly as: --password=env:xxx --password=file:xxx --password=pass:xxx thanks! Line 35: Line 36: // Query arguments Line 37: public static final String ARG_AUTHZ_NAME = "--authz"; Line 38: public static final String ARG_QUERY_ENTITY = "--query-entity"; Line 39: public static final String ARG_PAGE_SIZE = "--page-size"; Line 40: public static final String ARG_IDS = "--ids"; Line 41: public static final String ARG_NAME = "--name"; Line 42: public static final String ARG_RESOLVE_GROUPS_RECURSIVE = "--resolve-groups-recursive"; Line 43: public static final String ARG_MAX_QUERY_RESULTS = "--max-query-results"; I think better to have: --property=key=value instead of more and more parameters. Line 44: Line 45: // Query entity values Line 46: public static final String QUERY_ENTITY_PRINCIPAL = "principal"; Line 47: public static final String QUERY_ENTITY_GROUP = "group"; Line 238: parser.addArg(new ArgumentBuilder(). Line 239: longName(ARG_RESOLVE_GROUPS_RECURSIVE). Line 240: valueRequired(false). Line 241: build()); Line 242: I never understood why this is not metadata as resource... but it is the best I could reach with the author Line 243: } Line 244: return parser; Line 245: } Line 246: -- To view, visit http://gerrit.ovirt.org/27814 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ea2f9c62ced5bdd3801c9f6d8087a35e3c21886 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
