Skip to content
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 GCP package #459

Merged
merged 16 commits into from
Mar 5, 2021
Merged

Add GCP package #459

merged 16 commits into from
Mar 5, 2021

Conversation

marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Dec 14, 2020

What does this PR do?

Adds GCP package

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all datasets collect metrics or logs.

Screenshots

image

image

image

image

@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link

elasticmachine commented Dec 14, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #459 updated

  • Start Time: 2021-03-05T12:14:54.615+0000

  • Duration: 12 min 33 sec

  • Commit: 81927bb

Test stats 🧪

Test Results
Failed 0
Passed 23
Skipped 0
Total 23

Trends 🧪

Image of Build Times

Image of Tests

@andrewkroh
Copy link
Member

I like that you converted as much as possible to Ingest Node.

The Google pub/sub service has an emulator. Perhaps the emulator can be loaded with our json logs and then the e2e tests pull from the emulator's topics.

The reason the logfile input exists in these GCP data streams is to allow for testing. By including it in the UI I think we make the usage less clear. We might be able to remove it since there is an alternative means of testing via the emulator. However some different services (like s3+sqs) don't have a good emulation option so logfile is still needed IMO. It might be useful to have a way for an input to be available only for testing.

@marc-gr
Copy link
Contributor Author

marc-gr commented Dec 16, 2020

I like that you converted as much as possible to Ingest Node.

The Google pub/sub service has an emulator. Perhaps the emulator can be loaded with our json logs and then the e2e tests pull from the emulator's topics.

The reason the logfile input exists in these GCP data streams is to allow for testing. By including it in the UI I think we make the usage less clear. We might be able to remove it since there is an alternative means of testing via the emulator. However some different services (like s3+sqs) don't have a good emulation option so logfile is still needed IMO. It might be useful to have a way for an input to be available only for testing.

I will give it a try, and I opened an issue elastic/package-spec#97 for what you are mentioning.

@marc-gr
Copy link
Contributor Author

marc-gr commented Dec 18, 2020

We have to wait for the changes in elastic/beats#23215 to be able to use the emulator here

@andrewkroh
Copy link
Member

jenkins, run tests

Now that alternative_host has been merged to Filebeat let's see how this goes.

@andrewkroh
Copy link
Member

andrewkroh commented Jan 24, 2021

@marc-gr This needs an update to change config.yml to test-pubsub-config.yml. ref: https://github.com/elastic/package-spec/blob/6435a2f6b3e0b92c11ffb4531a0ad3f575093d3f/versions/1/data_stream/_dev/test/system/spec.yml#L6

@adriansr
Copy link
Contributor

adriansr commented Feb 19, 2021

For the logfile input, you may want to set enabled: false in the manifest.yml, so it's not enabled by default in the configuration.

Edit: nevermind, I see that you've dropped the input altogether.

@marc-gr marc-gr force-pushed the gcp_pkg branch 2 times, most recently from cccb5e9 to 512d905 Compare February 23, 2021 15:41
@mtojek mtojek self-requested a review February 24, 2021 09:49
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marc-gr I see that you pushed a lot of Go code here, not sure if this is the expected way to go. In general it would be better to depend on DSLs/manifests/configs.

If you need to create some resources on GCP, you can try to use the Terraform service deployer: https://github.com/elastic/elastic-package/blob/master/docs/howto/system_testing.md#terraform-service-deployer

Here is the appliance for AWS: https://github.com/elastic/integrations/tree/master/packages/aws/data_stream/ec2_metrics/_dev/deploy/tf

@marc-gr marc-gr force-pushed the gcp_pkg branch 3 times, most recently from 8786417 to 59bdf04 Compare March 3, 2021 10:29
@marc-gr
Copy link
Contributor Author

marc-gr commented Mar 3, 2021

@marc-gr I see that you pushed a lot of Go code here, not sure if this is the expected way to go. In general it would be better to depend on DSLs/manifests/configs.

If you need to create some resources on GCP, you can try to use the Terraform service deployer: https://github.com/elastic/elastic-package/blob/master/docs/howto/system_testing.md#terraform-service-deployer

Here is the appliance for AWS: https://github.com/elastic/integrations/tree/master/packages/aws/data_stream/ec2_metrics/_dev/deploy/tf

Finally we enhanced akroh/stream to support GCP pubsub as an output, so we moved the go code there and just using docker here 👍

@marc-gr marc-gr requested a review from mtojek March 3, 2021 10:34
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally we enhanced akroh/stream to support GCP pubsub as an output, so we moved the go code there and just using docker here 👍

Thanks for the information. Could you please share details behind this decision? I'm wondering if it's because of the fact that TF service deployer is not feasible for your use cases.

Another thought: do you plan to move akroh/stream under elastic org? I know it's Andrew's repository, but I believe in general we should depend on company's ownership.

@andrewkroh
Copy link
Member

andrewkroh commented Mar 3, 2021

Thanks for the information. Could you please share details behind this decision? I'm wondering if it's because of the fact that TF service deployer is not feasible for your use cases.

I think it would be valuable to have a test like this too. It would serve as an integration test against the real service and could catch different types of issues than the emulator based testing with static logs. If the TF setup configures the actual services to write logs that's even better since it would let us catch when log formats changes. And lastly it could serve as a pseudo reference for describing the configuration steps necessary to setup GCP to use the package.

Another thought: do you plan to move akroh/stream under elastic org? I know it's Andrew's repository, but I believe in general we should depend on company's ownership

I agree. I wasn't sure whether it was going to be useful at first so I opted for a personal repo where I could more independently setup CI. I'll work on getting this moved over and have Jenkins setup.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice work with all the testing improvements to allow the emulator to be used.

- --addr=gcppubsub-emulator:8681
- -p=gcppubsub
- --gcppubsub-clear=true
- --gcppubsub-project=audit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi: You can also pass any option via env vars:

environment:
  - STREAM_GCPPUBSUB_CLEAR=true
  - STREAM_GCPPUBSUB_PROJECT=audit

packages/gcp/data_stream/audit/manifest.yml Outdated Show resolved Hide resolved
go.sum Outdated Show resolved Hide resolved
@marc-gr marc-gr merged commit 5e8b422 into elastic:master Mar 5, 2021
@marc-gr marc-gr deleted the gcp_pkg branch March 5, 2021 12:39
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* Audit data stream

* Firewall dataset

* Add vpcflow dataset

* Manifests, dashboards and system tests

* Remove log input

* Add gcp emulator for system testing

* Rename system config files

* Add changelog

* Move all processors to ingest

* Add fake credentials

* Use go client to publish test messages

* Rename system tests service

* Change custom docker container for akroh/stream

* Use stream v0.3.0 for system tests

* Hide advanced settings

* Remove unnecessary config from deploy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants