[Filebeat] Add pubsub_alternative_host to gcp pubsub input #23215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marc-gr
added
enhancement
Filebeat
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
botelastic
bot
added
needs_team
2 tasks
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
andrewkroh
reviewed
Dec 18, 2020
| return nil, fmt.Errorf("grpc.Dial: %v", err) | ||
| } | ||
| opts = append(opts, option.WithGRPCConn(conn), option.WithTelemetryDisabled()) | ||
| } else if in.CredentialsFile != "" { |
There was a problem hiding this comment.
The PubsubAlternativeHost and Credentials* options are not mutually exclusive, right?
Suggested change
| } else if in.CredentialsFile != "" { | |
| } | |
| if in.CredentialsFile != "" { |
| @@ -36,6 +36,8 @@ type config struct { | |||
|
|
|||
| // JSON blob containing authentication credentials and key. | |||
| CredentialsJSON []byte `config:"credentials_json"` | |||
|
|
|||
| PubsubAlternativeHost string `config:"pubsub_alternative_host"` | |||
There was a problem hiding this comment.
I'm debating about the pubsub prefix. I thinking to remove it since the package is called pubsub and the in config it will be part of the gcp-pubsub input.
Suggested change
| PubsubAlternativeHost string `config:"pubsub_alternative_host"` | |
| // Overrides the default Pub/Sub service address and disables TLS. For testing. | |
| AlternativeHost string `config:"alternative_host"` |
andrewkroh
reviewed
Dec 18, 2020
| // this will be typically set because we want to point the input to a testing pubsub emulator | ||
| conn, err := grpc.Dial(in.PubsubAlternativeHost, grpc.WithInsecure()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("grpc.Dial: %v", err) |
There was a problem hiding this comment.
Suggested change
| return nil, fmt.Errorf("grpc.Dial: %v", err) | |
| return nil, fmt.Errorf("grpc.Dial: %w", err) |
marc-gr
force-pushed
the
pubsub-alt-host
branch
from
fa06828
to
6b36a30
Compare
adriansr
suggested changes
Dec 21, 2020
Comment on lines
15
to
17
| "google.golang.org/grpc" | ||
|
|
||
| "cloud.google.com/go/pubsub" | ||
| "github.com/pkg/errors" | ||
| "google.golang.org/api/option" |
There was a problem hiding this comment.
Suggested change
| "google.golang.org/grpc" | |
| "cloud.google.com/go/pubsub" | |
| "github.com/pkg/errors" | |
| "google.golang.org/api/option" | |
| "cloud.google.com/go/pubsub" | |
| "github.com/pkg/errors" | |
| "google.golang.org/api/option" | |
| "google.golang.org/grpc" |
| // this will be typically set because we want to point the input to a testing pubsub emulator | ||
| conn, err := grpc.Dial(in.AlternativeHost, grpc.WithInsecure()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("grpc.Dial: %w", err) |
There was a problem hiding this comment.
can we make this error more descriptive? "cannot connect to alternative_host" or similar
adriansr
approved these changes
Dec 21, 2020
marc-gr
force-pushed
the
pubsub-alt-host
branch
2 times, most recently
from
dc20b57
to
7a947e3
Compare
marc-gr
force-pushed
the
pubsub-alt-host
branch
from
7a947e3
to
5e9b674
Compare
3 tasks
marc-gr
removed
the
needs_backport
Merged
3 tasks
marc-gr
added a commit
that referenced
this pull request
Dec 22, 2020
marc-gr
added a commit
that referenced
this pull request
Dec 22, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds an option to set a custom pubsub alternative host to pubsub input.
Why is it important?
To be able to start the input using the GCP Pubsub emulator we needed a way to configure it to point to our testing host and bypass credential initialization.
Checklist
- [ ] I have made corresponding changes to the documentation(we do not want to document that as it is intended for testing)- [ ] I have made corresponding change to the default configuration files- [ ] I have added tests that prove my fix is effective or that my feature worksCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.