-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
add snowflake detector #1653
add snowflake detector #1653
Conversation
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.
in addition to my in-line comment, an indeterminate failure test would be nice if it's feasible
pkg/detectors/snowflake/snowflake.go
Outdated
databases = append(databases, name) | ||
} | ||
s1.ExtraData["databases"] = strings.Join(databases, ", ") | ||
s1.Verified = true |
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.
The presence of a verification error (which is possible above) signals that the detector wasn't able to determine whether the candidate credentials are valid. That's not really consistent with setting Verified
to true (which means that we know that the credentials are valid) - the framework assumes that you won't combine those two things, so you shouldn't. I'm not sure what the best solution here is - maybe putting those error messages somewhere else in the Result
payload?
(I know that making this fourth "verification state" mathematically possible but forbidding its use is kind of weird, but it's the price we pay for being able to migrate detectors gradually.)
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.
ahh - good point
Resolves #1607
Description:
Explain the purpose of the PR.
Checklist:
make test-community
)?make lint
this requires golangci-lint)?