redact sensitive data #3
Conversation
There was a problem hiding this comment.
Unrelated to this change, but func send(url string, t *model.Telemetry) error is not checking for proxy settings (http_proxy / https_proxy env) which is required for outgoing traffic in many corporate networks - this might make oasdiff feel slow while waiting for the timeout on each run.
There was a problem hiding this comment.
Great point, thanks for that! I'll create a new pull request for that.
BTW the telemetry logic in oasdiff runs as a separate go-routine with a timeout.
client/client.go
Outdated
|
|
||
| func redactFlags(telemetry *model.Telemetry) *model.Telemetry { | ||
|
|
||
| for key := range telemetry.Flags { | ||
| if key == "err-ignore" || | ||
| key == "warn-ignore" || | ||
| key == "match-path" || | ||
| key == "prefix-base" || | ||
| key == "prefix-revision" || | ||
| key == "strip-prefix-base" || | ||
| key == "strip-prefix-revision" || | ||
| key == "filter-extension" { | ||
| telemetry.Flags[key] = "" | ||
| } | ||
| } |
There was a problem hiding this comment.
The list of flags looks good to me. I would suggest passing the ignoreFlags to the telemetry client from the oasdiff cli when setting up the telemetry client - this makes it easier to maintain when adding or removing cli flags in the future as this is easy to miss without references in the main repo.
In that case this ignoreFlags list would move into oasdiff when telemetry is updated.
--
If you want to keep everything in the telemetry repo, a possible alternative could be to invert it and define the relevantFlags (instead of ignoreFlags ) in here, that way there is less risk when adding new flags.
| func redactFlags(telemetry *model.Telemetry) *model.Telemetry { | |
| for key := range telemetry.Flags { | |
| if key == "err-ignore" || | |
| key == "warn-ignore" || | |
| key == "match-path" || | |
| key == "prefix-base" || | |
| key == "prefix-revision" || | |
| key == "strip-prefix-base" || | |
| key == "strip-prefix-revision" || | |
| key == "filter-extension" { | |
| telemetry.Flags[key] = "" | |
| } | |
| } | |
| var ignoreFlags = []string{ | |
| "err-ignore", | |
| "warn-ignore", | |
| "match-path", | |
| "prefix-base", | |
| "prefix-revision", | |
| "strip-prefix-base", | |
| "strip-prefix-revision", | |
| "filter-extension", | |
| } | |
| func redactFlags(telemetry *model.Telemetry) *model.Telemetry { | |
| for _, key := range ignoreFlags { | |
| if _, exists := telemetry.Flags[key]; exists { | |
| telemetry.Flags[key] = "" | |
| } | |
| } |
There was a problem hiding this comment.
done, see: config collector with ignore-flags
| func getWebCategory(name string) string { | ||
|
|
||
| if isSwaggerHub(name) { | ||
| return "swaggerhub" | ||
| } else if isGitHub(name) { | ||
| return "github" | ||
| } else if isGCS(name) { | ||
| return "gcs" | ||
| } else if isS3(name) { | ||
| return "s3" | ||
| } else if isAzure(name) { | ||
| return "azure" | ||
| } else if isHeroku(name) { | ||
| return "heroku" | ||
| } else if strings.HasPrefix(name, "https://") { | ||
| return "https" | ||
| } else if strings.HasPrefix(name, "http://") { | ||
| return "http" | ||
| } | ||
|
|
||
| return "" | ||
| } |
There was a problem hiding this comment.
I think file, swaggerhub and http/https would suffice for the source.
If other protocols like S3 / FTP, ... are supported those could also be added.
There was a problem hiding this comment.
What do you mean by supported?
I'm working with GCS for storing OAS
There was a problem hiding this comment.
I think of file and http/https as supported protocols and swaggerhub as special case of interest.
I don't see how knowing what hosters are used (azure/heroku/...) is useful to oasdiff and therefore merge all of them into http/https. If you think that this information is useful keeping it would be ok as well - the important part was removing the full url.
With supported protocols i mean cases that are not a plain http requests, to give an example for s3:
Imagine oasdiff accepting a argument like s3://<bucket-name>.s3.amazonaws.com/specs to compare all files in a local directory with all files in a bucket in composed mode.
Co-authored-by: Philipp Heuer <[email protected]>
No description provided.