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 handling of multiple log destinations #2328

Merged
merged 1 commit into from
May 10, 2022
Merged

Add handling of multiple log destinations #2328

merged 1 commit into from
May 10, 2022

Conversation

rafwegv
Copy link
Contributor

@rafwegv rafwegv commented Jan 4, 2022

Proposed changes

Add a field to policy crd which allows for specifying multiple log destinations. If the old field is used with the new field the old field is ignored. Otherwise old OR new fields work

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 master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@nginx-bot nginx-bot force-pushed the ap-mutli-log-vs branch 5 times, most recently from 1f52096 to f732580 Compare January 7, 2022 18:42
@brianehlert brianehlert added the nap label Jan 7, 2022
@nginx-bot nginx-bot removed the nap label Jan 7, 2022
@github-actions github-actions bot added the documentation Pull requests/issues for documentation label Jan 18, 2022
@rafwegv rafwegv self-assigned this Jan 18, 2022
@rafwegv rafwegv changed the title WIP: Add handling of mutiple log destinations Add handling of multiple log destinations Jan 18, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2022

Codecov Report

Merging #2328 (d4d6bce) into main (8706d5d) will increase coverage by 0.03%.
The diff coverage is 88.52%.

❗ Current head d4d6bce differs from pull request most recent head 83897e9. Consider uploading reports for the commit 83897e9 to get more accurate results

@@            Coverage Diff             @@
##             main    #2328      +/-   ##
==========================================
+ Coverage   53.47%   53.51%   +0.03%     
==========================================
  Files          52       52              
  Lines       14696    14729      +33     
==========================================
+ Hits         7859     7882      +23     
- Misses       6577     6583       +6     
- Partials      260      264       +4     
Impacted Files Coverage Δ
internal/configs/version2/http.go 0.00% <ø> (ø)
internal/configs/virtualserver.go 95.13% <83.33%> (-0.19%) ⬇️
internal/k8s/controller.go 11.38% <89.28%> (+0.51%) ⬆️
internal/configs/configurator.go 37.48% <100.00%> (+0.03%) ⬆️
internal/configs/ingress.go 76.49% <100.00%> (-0.06%) ⬇️
internal/k8s/configuration.go 95.47% <100.00%> (-0.39%) ⬇️
pkg/apis/configuration/validation/virtualserver.go 94.74% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@nginx-bot nginx-bot force-pushed the ap-mutli-log-vs branch 6 times, most recently from 10d8fb9 to ac21b74 Compare January 20, 2022 07:41
@ciarams87 ciarams87 requested review from ciarams87 and removed request for soneillf5 January 20, 2022 14:12
Copy link
Member

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Left a couple of comments, also one of the files is not properly formatted, that's why there's a linting error

internal/configs/virtualserver.go Show resolved Hide resolved
tests/suite/test_app_protect_waf_policies.py Outdated Show resolved Hide resolved
@lucacome lucacome requested a review from a team March 24, 2022 06:01
@lucacome lucacome requested a review from haywoodsh March 24, 2022 06:01
@lucacome lucacome added the enhancement Pull requests for new features/feature enhancements label Apr 13, 2022
@rafwegv rafwegv force-pushed the ap-mutli-log-vs branch 2 times, most recently from d4d6bce to 83897e9 Compare April 25, 2022 16:07
@rafwegv rafwegv force-pushed the ap-mutli-log-vs branch from 426ba6e to 42ec1a7 Compare May 9, 2022 14:39
@rafwegv rafwegv force-pushed the ap-mutli-log-vs branch from 42ec1a7 to 2ae0333 Compare May 10, 2022 09:08
@rafwegv rafwegv merged commit 79b8c9c into main May 10, 2022
@rafwegv rafwegv deleted the ap-mutli-log-vs branch May 10, 2022 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants