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.

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

fix: Remove Server header and allow all on port 80 #29585

Merged
merged 11 commits into from
Dec 18, 2023
Merged

fix: Remove Server header and allow all on port 80 #29585

merged 11 commits into from
Dec 18, 2023

Conversation

sharat87
Copy link
Member

@sharat87 sharat87 commented Dec 13, 2023 •
edited
Loading

Another attempt at #29550, which was reverted. Fallback is not happening if cert provisioning fails despite having the correct header. But with the changes in this PR, since we'll listen on :80, fallback will happen when cert provisioning fails due to incorrect domain configuration.

We're also adding Hurl based tests. They're not run in any CI yet. That'll come in soon.

Summary by CodeRabbit

  • Refactor

    • Improved handling of custom domain configurations in the server setup.
    • Enhanced HTTP to HTTPS redirection for custom domains.

  • Chores

    • Updated environment variable naming for clarity in custom domain setup.

@sharat87
Copy link
Member Author

/build-deploy-preview skip-tests=true

@github-actions github-actions bot added the Bug Something isn't working label Dec 13, 2023
@github-actions GitHub Actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7196421276.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 29585.
recreate: .

@github-actions GitHub Actions
Copy link

Deploy-Preview-URL: https://ce-29585.dp.appsmith.com

@sharat87
Copy link
Member Author

/build-deploy-preview skip-tests=true

@github-actions GitHub Actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7197286462.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 29585.
recreate: .

@github-actions GitHub Actions
Copy link

Deploy-Preview-URL: https://ce-29585.dp.appsmith.com

@sharat87
Copy link
Member Author

/build-deploy-preview skip-tests=true

@github-actions GitHub Actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7208698121.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 29585.
recreate: .

@github-actions GitHub Actions
Copy link

Deploy-Preview-URL: https://ce-29585.dp.appsmith.com

@sharat87
Copy link
Member Author

/build-deploy-preview skip-tests=true

@github-actions GitHub Actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7210491881.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 29585.
recreate: .

@github-actions GitHub Actions
Copy link

Deploy-Preview-URL: https://ce-29585.dp.appsmith.com

@sharat87 sharat87 changed the title fix: Fallback to HTTP, if cert provisioning fails, take 2 fix: Remove Server header and allow all on port 80 Dec 15, 2023
@sharat87
Copy link
Member Author

/ok-to-test

@github-actions GitHub Actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/7218974001.
Workflow: Appsmith External Integration Test Workflow.
Tags: ``.

@github-actions GitHub Actions
Copy link

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7218974001.
Commit: ``.
Cypress dashboard url: Click here!
All cypress tests have passed 🎉🎉🎉

@sharat87 sharat87 marked this pull request as ready for review December 15, 2023 09:17
@coderabbitai coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2023 •
edited
Loading

Warning

Rate Limit Exceeded

@sharat87 has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 9 minutes and 12 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 805985f and 0f6fda1.

Walkthrough

The changes across the files primarily revolve around enhancing the handling of custom domains, HTTP to HTTPS redirection, and SSL certificate management. Additionally, adjustments have been made in the processing and manipulation of forwarded headers in HTTP requests.

Changes

File Change Summary
deploy/docker/fs/opt/.../caddy-reconfigure.mjs
deploy/docker/route-tests/Dockerfile		 Updated handling of custom domains, HTTP to HTTPS redirection, and SSL certificate management.
deploy/docker/route-tests/common-https/forwarded-headers.hurl
deploy/docker/route-tests/common/forwarded-headers.hurl		 Modified logic for processing and manipulating forwarded headers in HTTP requests.
deploy/docker/route-tests/common/static-404.hurl Added test cases for handling 404 responses and absence of "Server" header in HTTP responses.
deploy/docker/route-tests/echo.caddyfile Added configuration for an HTTP server and response template.
deploy/docker/route-tests/entrypoint.sh Updated test scenarios and setup of custom domains with SSL certificates.
deploy/docker/route-tests/spec-2/custom-domain.hurl HTTP request-response tests for a custom domain with minor URL and response modifications.

Related issues

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@sharat87 sharat87 merged commit e6ebfba into release Dec 18, 2023
13 checks passed
@sharat87 sharat87 deleted the chore/caddy-ssl-fix branch December 18, 2023 04:14
sharat87 added a commit that referenced this pull request Dec 23, 2023
@sharat87
fix: Remove Server header and allow all on port 80 (#29585)
4de225d 
Another attempt at #29550, which was reverted. Fallback is not happening
if cert provisioning fails _despite_ having the correct header. But with
the changes in this PR, since we'll listen on `:80`, fallback _will_
happen when cert provisioning fails due to incorrect domain
configuration.

We're also adding [Hurl](https://hurl.dev) based tests. They're not run
in any CI yet. That'll come in soon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai coderabbitai left review comments

@mohanarpit mohanarpit mohanarpit approved these changes

@pratapaprasanna pratapaprasanna Awaiting requested review from pratapaprasanna pratapaprasanna is a code owner

Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sharat87 @mohanarpit