Skip to content

Conversation

@vidarl
Copy link
Contributor

@vidarl vidarl commented Oct 24, 2024

Question Answer
JIRA Ticket N/A
Versions 4.6
Edition any

Instructions on how to provide basic auth credentials in purge request was missing

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Description metadata is up to date
  • Redirects cover removed/moved pages
  • Code samples are working
  • PHP code samples have been fixed with PHP CS fixer
  • Added link to this PR in relevant JIRA ticket or code PR

@vidarl vidarl requested a review from adriendupuis October 24, 2024 13:44
@vidarl vidarl force-pushed the varnish_basic_auth branch from 6a746b4 to 8171be8 Compare October 24, 2024 13:48
Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

Thank you Vidar! Just a couple of small suggestions from me


``` yaml
http_cache:
purge_servers: [http://myuser:[email protected]:8081]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
purge_servers: [http://myuser:[email protected]:8081]
purge_servers: [https://myuser:[email protected]:8081]

Can we suggest using HTTPS by default, for everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do that. But open source version of Varnish do not support HTTPS termiation, so I think it makes more sense to always use HTTP for varnish purge servers in or docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point! My mind always goes to "HTTPS links everywhere", but it makes sense to make an exception when it's expected to see HTTP.

@mnocon mnocon merged commit 981c2f5 into 4.6 Oct 29, 2024
5 of 6 checks passed
@mnocon mnocon deleted the varnish_basic_auth branch October 29, 2024 13:48
mnocon added a commit that referenced this pull request Oct 29, 2024
* Added instructions on how to combine varnish with basic-auth

* fixup! Added instructions on how to combine varnish with basic-auth

* Apply suggestions from code review

Co-authored-by: Marek Nocoń <[email protected]>

---------

Co-authored-by: Marek Nocoń <[email protected]>
@mnocon
Copy link
Contributor

mnocon commented Oct 29, 2024

I've also upmerged it to master - thank you for the PR Vidar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants