-
Notifications
You must be signed in to change notification settings - Fork 126
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
Pass domains as arguments similar to dig #418
Conversation
It really feels offhand like the module should come first. It's very confusing to me to allow it to be in the middle or end. |
@zakird I don't think I have strong preferences either way. A lookup module name will never be confused with a domain name, (A is not a valid domain, for example) so I'm not worried about difficulties in parsing it or mistaking a module name for a domain or vice versa. |
src/cli/cli.go
Outdated
"MR", "MX", "MXLOOKUP", "NAPTR", "NID", "NIMLOC", "NINFO", "NS", "NSAPPTR", "NSEC", "NSEC3", "NSEC3PARAM", | ||
"NSLOOKUP", "NULL", "NXT", "OPENPGPKEY", "PTR", "PX", "RP", "RRSIG", "RT", "SMIMEA", "SOA", "SPF", "SRV", | ||
"SSHFP", "SVCB", "TALINK", "TKEY", "TLSA", "TXT", "UID", "UINFO", "UNSPEC", "URI", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables get initialized and then init() gets run (where modules are registered), so we'll have to hardcode the list.
Why can't we just populate the variable at the time of init() running. I don't follow why this needs to be in the variable declaration. This feels like it's absolutely going to end up out of date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to say earlier when I replied to the other one, I took another look at this and found another way to go about it that doesn't require this. I agree that the hard-coded list was asking for trouble.
"MXLOOKUP": {}, | ||
"NSLOOKUP": {}, | ||
} | ||
|
There was a problem hiding this comment.
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 they shouldn't be printed as part of modules. It seems like we want the user to know they exist? They are modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're modules in the way we mean it with ZDNS but in terms of Cobra/the CLI, they're commands. We register them as "commands" and they get printed by Cobra here:
$ ./zdns --help
...
Usage:
zdns [flags]
zdns [command]
Available Commands:
completion Generate the autocompletion script for the specified shell
help Help about any command
mxlookup Run a more exhaustive mxlookup
nslookup Run a more exhaustive nslookup
So they must be called with ./zdns mxlookup
., or ./zdns_binary CMD_NAME
We can't have ./zdns --module=mxlookup
since Cobra only initializes the command specific flags (--mx-cache-size
here) if the command is used as cobra expects.
So I'd say they are modules in the ZDNS sense but we must call them like Cobra expects commands to be called. Cobra will print out mxlookup
and nslookup
in the Available Commands
section, and we'll print out the other ZDNS modules in the new section of --help
, Available modules:
Available modules:
A AAAA AFSDB ANY ATMA AVC
AXFR BINDVERSION CAA CDNSKEY CDS CERT
CNAME CSYNC DHCID DMARC DNAME DNSKEY
DS EID EUI48 EUI64 GID GPOS
HINFO HIP HTTPS ISDN KEY KX
L32 L64 LOC LP MB MD
MF MG MR MX NAPTR NID
NIMLOC NINFO NS NSAPPTR NSEC NSEC3
NSEC3PARAM NULL NXT OPENPGPKEY PTR PX
RP RRSIG RT SMIMEA SOA SPF
SRV SSHFP SVCB TALINK TKEY TLSA
TXT UID UINFO UNSPEC URI
In this way, the user will know about all available options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not just define our own --help
that patches this up? Does cobra allow us to define that ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure how that would solve things.
func init() {
rootCmd.AddCommand(mxlookupCmd)
mxlookupCmd.PersistentFlags().Bool("ipv4-lookup", false, "perform A lookups for each MX server")
mxlookupCmd.PersistentFlags().Bool("ipv6-lookup", false, "perform AAAA record lookups for each MX server")
mxlookupCmd.PersistentFlags().Int("mx-cache-size", 1000, "number of records to store in MX -> A/AAAA cache")
util.BindFlags(mxlookupCmd, viper.GetViper(), util.EnvPrefix)
}
The --ipv4-lookup
and --ipv6-lookup
don't matter too much because we also define those on the rootCmd
, but the mx-cache-size
is only defined here.
Cobra will only initialize that flag if the mxlookup
command is used as a "Cobra command", ie. zdns mxlookup
.
Unless we define --mx-cache-size
on the root command and let it be used by all modules, we must treat it separately because Cobra handles it differently.
Closing, will re-work post-ZFlags merge |
Closes #266
Description
Let's the user provide domains as additional arguments to ZDNS, ex:
./zdns --module=A google.com yahoo.com
as long as they provide the module with--module
.--module
echo "Cloudflare.com" | ./zdns A
)--name-server-mode
--help
to show available modulesTesting
Old Behavior Unchanged
Module can be specified with
--module
User can provide domains with a module
--name-server-mode
--name-server-mode
with dig-style inputsArg. Order Doesn't Matter
Error States
User must specify type
Multiple types not allowed