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.

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

Implement new 'discover' command #109

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Implement new 'discover' command #109

wants to merge 2 commits into from

Conversation

AbGuthrie
Copy link
Owner

image

  • This new command queries the api server and asks for a list of available known hosts.
  • Also tidy up lint errors for struct initialization with no explicit field names

This PR closes #93

Copy link
Collaborator

@obelisk obelisk left a comment

Choose a reason for hiding this comment

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

This should probably also be able to filter somehow otherwise this would be a crazy call on large deployments.

http.Handle("/scheduleQuery", samlSP.RequireAccount(sq))
http.Handle("/fetchResults", samlSP.RequireAccount(fr))
http.Handle("/checkHost", samlSP.RequireAccount(_checkHost))
http.Handle("/listHosts", _listHosts)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be protected by SAML

@obelisk obelisk added enhancement New feature or request New API For issues or PRs that add functionality to goquery the requires new backend support new command For issues or PRs that add new commands to goquery labels Dec 14, 2019
@obelisk obelisk added this to the 2.0 Release milestone Dec 14, 2019
@AbGuthrie
Copy link
Owner Author

This should probably also be able to filter somehow otherwise this would be a crazy call on large deployments.

Any ideas on the parameters this might take? This will be a part of the spec contract a 'goserver'/mock server needs to implement. I'm thinking either a query param for "search" working as a glob search on any of the host fields, or a set of limit + offset parameters.

@@ -10,6 +10,7 @@ import (
// is blind to the implementation for code separation purposes.
type GoQueryAPI interface {
CheckHost(string) (hosts.Host, error)
ListHosts() (utils.Rows, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this is utils.Rows and not []hosts.Host?

Copy link
Owner Author

@AbGuthrie AbGuthrie Jan 15, 2020

Choose a reason for hiding this comment

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

Hi Zachary, it is because this branch is out of date with master, which recently had the big rewrite

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sorry I missed the point of your question. No there is no reason, this was implemented as if this function expected the standard query response rows structure from the server. Since it is a specific goquery endpoint then yes, you are right, the shape of returned JSON from the server (or whatever is implementing this) should be of type []hosts.Host
Thanks for the catch, will update

@AbGuthrie AbGuthrie removed this from the 2.0 Release milestone Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request New API For issues or PRs that add functionality to goquery the requires new backend support new command For issues or PRs that add new commands to goquery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search For Hosts From Within goquery
3 participants