8000 Remove app protect agent by jjngx · Pull Request #3646 · nginx/kubernetes-ingress · GitHub
[go: up one dir, main page]
Skip to content

Remove app protect agent #3646

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Mar 20, 2023
Merged

Remove app protect agent #3646

merged 10 commits into from
Mar 20, 2023

Conversation

jjngx
Copy link
Contributor
@jjngx jjngx commented Mar 14, 2023

Proposed changes

NIC NAP (app protect) does not need app agent anymore. The only one process required to process policies and bundles is app protect plugin.

This PR:

  • removes references to app protect agent
  • update a list of app protect packages installed in docker containers

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork
8000

@jjngx jjngx requested a review from a team as a code owner March 14, 2023 16:32
@github-actions github-actions bot added the enhancement Pull requests for new features/feature enhancements label Mar 14, 2023
brianehlert
brianehlert reviewed Mar 14, 2023
View reviewed changes
ciarams87
ciarams87 requested changes Mar 14, 2023
View reviewed changes
@codecov-commenter
Copy link
codecov-commenter commented Mar 15, 2023 •
edited by codecov bot
Loading

Codecov Report

Merging #3646 (d5cdca5) into main (a2ad492) will increase coverage by 0.11%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #3646      +/-   ##
==========================================
+ Coverage   52.21%   52.32%   +0.11%     
==========================================
  Files          59       59              
  Lines       16878    16850      -28     
==========================================
+ Hits         8813     8817       +4     
+ Misses       7768     7738      -30     
+ Partials      297      295       -2
Impacted Files Coverage Δ
cmd/nginx-ingress/main.go 0.00% <0.00%> (ø)
internal/nginx/fake_manager.go 0.00% <0.00%> (ø)
internal/nginx/manager.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@vepatel vepatel requested a review from ciarams87 March 16, 2023 10:03
vepatel
vepatel approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor
@vepatel vepatel left a comment

Choose a reason for hiding this comment

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

passed AP tests 👍🏼

ciarams87
ciarams87 reviewed Mar 16, 2023
View reviewed changes
ciarams87
ciarams87 requested changes Mar 16, 2023
View reviewed changes
Copy link
Contributor
@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

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

See comment - we need to retain the logic for setting the AP log level.

@vepatel vepatel requested a review from ciarams87 March 16, 2023 15:29
@jjngx jjngx requested a review from a team March 20, 2023 10:31
@jjngx jjngx added this to the v3.1.0 milestone Mar 20, 2023
@jjngx jjngx merged commit cc61703 into main Mar 20, 2023
@jjngx jjngx deleted the feat/bundle branch March 20, 2023 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianehlert brianehlert brianehlert left review comments

@ciarams87 ciarams87 ciarams87 approved these changes

@vepatel vepatel vepatel approved these changes

+1 more reviewer

@lucacome lucacome lucacome left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jjngx @codecov-commenter @lucacome @brianehlert @ciarams87 @vepatel
0