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 access control to varnish #7700

Merged

Conversation

AbdelrahmanElawady
Copy link
Contributor

@AbdelrahmanElawady AbdelrahmanElawady commented Aug 4, 2023

Add access control to Varnish server with the same rules as TS.


Which Traffic Control components are affected by this PR?

  • Traffic Control Cache Config (t3c, formerly ORT)

What is the best way to verify this PR?

following the same steps in #7669

  • if Varnish is running as edge, try adding -X "PURGE" to curl command and it should return not allowed.
  • if Varnish is running as mid, same thing as edge but change the domain to mid-01.infra.ciab.test. it will return not allowed too.
  • you can also check VCL output at /opt/cache/etc/varnish/default.vcl

PR submission checklist

Fixes: #7714

@AbdelrahmanElawady
Copy link
Contributor Author

marked as draft until #7669 is merged

@AbdelrahmanElawady AbdelrahmanElawady marked this pull request as ready for review August 14, 2023 18:22
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #7700 (ec64608) into master (fe12a38) will increase coverage by 2.49%.
Report is 20 commits behind head on master.
The diff coverage is 74.12%.

@@             Coverage Diff              @@
##             master    #7700      +/-   ##
============================================
+ Coverage     26.88%   29.38%   +2.49%     
  Complexity       98       98              
============================================
  Files           691      594      -97     
  Lines         81154    74530    -6624     
  Branches         90       90              
============================================
+ Hits          21817    21897      +80     
+ Misses        57261    50550    -6711     
- Partials       2076     2083       +7     
Flag Coverage Δ
golib_unit 53.13% <74.12%> (+5.12%) ⬆️
grove_unit 12.02% <ø> (+7.41%) ⬆️
t3c_unit 5.99% <ø> (+1.08%) ⬆️
traffic_monitor_unit 26.22% <ø> (+4.92%) ⬆️
traffic_ops_unit 22.38% <ø> (+0.22%) ⬆️
traffic_stats_unit 10.76% <ø> (+0.62%) ⬆️
unit_tests 26.13% <74.12%> (+2.52%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
lib/varnishcfg/backends.go 71.07% <ø> (-2.01%) ⬇️
lib/varnishcfg/vcl.go 17.39% <0.00%> (+17.39%) ⬆️
lib/varnishcfg/vclbuilder.go 10.52% <0.00%> (+10.52%) ⬆️
lib/go-atscfg/ipallowdotyaml.go 74.53% <65.97%> (+2.91%) ⬆️
lib/varnishcfg/access_control.go 96.10% <96.10%> (ø)

... and 106 files with indirect coverage changes

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

@AbdelrahmanElawady AbdelrahmanElawady added new feature A new feature, capability or behavior low impact affects only a small portion of a CDN, and cannot itself break one cdn-in-a-box related to the Docker-based CDN-in-a-Box system cache-config Cache config generation low difficulty the estimated level of effort to resolve this issue is low Varnish related to Varnish labels Aug 16, 2023
Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@zrhoffman zrhoffman removed the low difficulty the estimated level of effort to resolve this issue is low label Aug 25, 2023
@zrhoffman zrhoffman merged commit 2a220b4 into apache:master Aug 25, 2023
33 checks passed
@AbdelrahmanElawady AbdelrahmanElawady deleted the add-access-control-varnish branch August 25, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cache-config Cache config generation cdn-in-a-box related to the Docker-based CDN-in-a-Box system low impact affects only a small portion of a CDN, and cannot itself break one new feature A new feature, capability or behavior Varnish related to Varnish
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add access control to Varnish cache server
2 participants