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

[Azure Blob Storage] Allowing empty content encoding headers for system tests #34412

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

P1llus
Copy link
Member

@P1llus P1llus commented Jan 30, 2023

What does this PR do?

Adding a failsafe for when content encoding headers might be nil, especially for system tests.

Why is it important?

Required for Agent integration package system tests

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

Testing instructions

When creating system tests for elastic-package, we use the Azure Blob Storage emulator: https://hub.docker.com/_/microsoft-azure-storage-azurite

Unfortunately the emulator itself does not return a ContentEncoding header, while the Cloud API will always ensure at least a default octet-stream is added if value is nil, we need to add the possibility for the Encoding Header to not be present to be able to continue the system tests.

Steps to reproduce:

  1. Start up the emulator: docker run -p 10000:10000 mcr.microsoft.com/azure-storage/azurite azurite-blob --blobHost 0.0.0.0
  2. Download stream and run go build : https://github.com/elastic/stream
  3. Create a local file for testing, for example test.json, add a JSON object on a single line, like { "testmessage" : "success" }
  4. Populate the emulator with the testdata using stream: ./stream log --retry=30 --addr=localhost -p azureblobstorage --azure-blob-storage-port=10000 --azure-blob-storage-container=azure-container1 ./test.json
  5. Configure the filebeat input to test against the emulator The password is a public key, not pasted by mistake:
filebeat.inputs:
- type: azure-blob-storage
  id: my-azureblobstorage-id
  enabled: true
  storage_url: http://localhost:10000/devstoreaccount1/
  account_name: devstoreaccount1
  auth.shared_credentials.account_key: Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==
  max_workers: 10
  poll: true
  poll_interval: 20s
  containers:
  - name: azure-container1
    max_workers: 1
    poll: true
    poll_interval: 20s

Before this PR, when we run filebeat with ./filebeat -e it will result in a panic:

panic: runtime error: invalid memory address or nil pointer dereference","component"
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x561c9cc3ac49]

However with this PR, the test will pass.

@P1llus P1llus requested a review from a team as a code owner January 30, 2023 08:51
@P1llus P1llus requested review from rdner and fearful-symmetry and removed request for a team January 30, 2023 08:51
@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jan 30, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 30, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @P1llus? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@mergify mergify bot assigned P1llus Jan 30, 2023
Copy link
Contributor

@ShourieG ShourieG left a comment

Choose a reason for hiding this comment

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

LGTM

@P1llus P1llus added backport-v8.6.0 Automated backport with mergify enhancement labels Jan 30, 2023
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 30, 2023

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-30T08:53:54.714+0000

  • Duration: 74 min 33 sec

Test stats 🧪

Test Results
Failed 0
Passed 2547
Skipped 172
Total 2719

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Member

@rdner rdner left a comment

Choose a reason for hiding this comment

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

Could you please add instructions to the PR description how this new behaviour was tested?

@P1llus P1llus merged commit 70f9ae3 into elastic:main Jan 30, 2023
mergify bot pushed a commit that referenced this pull request Jan 30, 2023
…em tests (#34412)

* Allowing empty content encoding headers for system tests

* update changelog

(cherry picked from commit 70f9ae3)
P1llus added a commit that referenced this pull request Jan 30, 2023
…em tests (#34412) (#34414)

* Allowing empty content encoding headers for system tests

* update changelog

(cherry picked from commit 70f9ae3)

Co-authored-by: Marius Iversen <marius.iversen@elastic.co>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
…em tests (#34412)

* Allowing empty content encoding headers for system tests

* update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.6.0 Automated backport with mergify enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants