packetbeat/beater: make Npcap installation lazy #35935
Merged
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
efd6
added
bug
Packetbeat
Team:Security-External Integrations
8.10-candidate
backport-v8.9.0
botelastic
bot
added
needs_team
efd6
force-pushed
the
agent-npcap-non-install
branch
from
5aaa950
to
ad90cad
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
bhapas
reviewed
Jul 13, 2023
| @@ -213,7 +213,7 @@ func TestCanInstallNpcap(t *testing.T) { | |||
| BeatConfig: cfg, | |||
| Manager: boolManager{managed: test.managed}, | |||
| } | |||
| got, err := canInstallNpcap(b) | |||
| got, err := canInstallNpcap(b, b.BeatConfig) | |||
There was a problem hiding this comment.
Minor thought : Can we pass cfg instead? Just to avoid the minor confusion that https://github.com/elastic/beats/pull/35935/files#diff-69402794a921b13ab8019aa16a00528596ce02e9ab1b40d7d266c44cc0bed664L122 has changed b.BeatConfig to rawCfg
There was a problem hiding this comment.
Yeah, OK. It shouldn't be any more confusing than it already is.
This comment was marked as outdated.
This comment was marked as outdated.
In the fleet-managed case New is called without a complete config, which means that we have no stream to tell us not to install the Npcap DLL. Defer installation until we have a valid config with streams configured so that we can determine whether the user has blocked install and whether we need the DLL.
efd6
force-pushed
the
agent-npcap-non-install
branch
from
d714a57
to
2c0f85c
Compare
efd6
force-pushed
the
agent-npcap-non-install
branch
3 times, most recently
from
1987a00
to
3c117c0
Compare
efd6
force-pushed
the
agent-npcap-non-install
branch
from
3c117c0
to
55ad07f
Compare
kcreddy
approved these changes
Aug 1, 2023
There was a problem hiding this comment.
LGTM 👍🏼
Following tests were run:
npcapisn't getting installed ifnever_install: truein Network Packet Integration.- Packetbeat throws error when there is no pre-existing
npcapdll, and honors the existingnpcapversion if it was already installed on the host.
- Packetbeat throws error when there is no pre-existing
npcapis correctly getting installed from the provided bundle ifnever_install: falseand the host doesn't contain priornpcapinstallation.
mergify bot
pushed a commit
that referenced
this pull request
Aug 1, 2023
Defer installation until we have a valid config with streams configured so that we can determine whether the user has blocked install and whether we need the DLL. In the fleet-managed case New is called without a complete config, which means that we have no stream to tell us not to install the Npcap DLL. (cherry picked from commit 0a655a1)
efd6
added a commit
that referenced
this pull request
Aug 1, 2023
#36190) Defer installation until we have a valid config with streams configured so that we can determine whether the user has blocked install and whether we need the DLL. In the fleet-managed case New is called without a complete config, which means that we have no stream to tell us not to install the Npcap DLL. (cherry picked from commit 0a655a1) Co-authored-by: Dan Kortschak <[email protected]>
Scholar-Li
pushed a commit
to Scholar-Li/beats
that referenced
this pull request
Feb 5, 2024
Defer installation until we have a valid config with streams configured so that we can determine whether the user has blocked install and whether we need the DLL. In the fleet-managed case New is called without a complete config, which means that we have no stream to tell us not to install the Npcap DLL.
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?
Defer installation until we have a valid config with streams configured so that we can determine whether the user has blocked install and whether we need the DLL.
Why is it important?
In the fleet-managed case New is called without a complete config, which means that we have no stream to tell us not to install the Npcap DLL.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs