-
Notifications
You must be signed in to change notification settings - Fork 122
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
feat: add fedramp flag & remove gov license prefix #286
Conversation
Pull Request Test Coverage Report for Build 498128478
💛 - Coveralls |
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.
Really nice, I like when a change is easy to make. ✅
Does not solve #278, fedramp config miss to setup log url fedramp.
pkg/integrations/v4/logs/cfg.go
Outdated
if cfg.IsFedramp { | ||
ret.Endpoint = fedrampEndpoint | ||
} | ||
|
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.
IMO order of precedence would be: stg, fedramp and last euEndpoint.
So I would move
if license.IsRegionEU(cfg.License) {
ret.Endpoint = euEndpoint
}
Should be first conditional.
(I don't know how to request a change 😿)
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.
LGTM
fedramp: true
flag.gov
license key prefix handling, as there are no such prefixes.Details on endpoints: https://docs.newrelic.com/docs/security/security-privacy/compliance/fedramp-compliant-endpoints
Solves #278