I've been doing this in pieces bit by bit, not to rock the boat too much
too fast, but it's taking too long, and too much effort. Instead opting
for a rip-the-bandaid style, hopefully without the pain.
Instead of downloading it from the github repo on every build (in
`base.dockerfile`), we vendor the supervisor extension so we can look to
fix the buffer overflow problem in supervisor logs.
After this is merged, after a day/two, we'll be removing `git`,
`python3-pip` and the supervisor-stdout extension from
`base.dockerfile`.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Introduced a Databricks plugin for executing queries and managing
database connections.
- Added a migration to incorporate the Databricks plugin into existing
workspaces.
- **Bug Fixes**
- Ensured robust error handling in the Databricks plugin with clear
messaging for query execution failures.
- **Tests**
- Implemented tests to validate the behavior of the Databricks plugin
under various connection scenarios.
- **Documentation**
- Included configuration properties for the Databricks plugin setup.
- **Refactor**
- Added specific error types and messages for the Databricks plugin to
improve debugging and user feedback.
- **Chores**
- Modified the Java runtime environment settings to support the new
plugin's requirements.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Arpit Mohan <arpit@appsmith.com>
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.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Updated the source URL for Docker Compose file download in the cloud
initialization script to a specific GitHub repository for improved
reliability and traceability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Defining custom domain as `https://example.com/` is invalid.
It should be just the domain, just `example.com`. But turns out a lot of
our users have the incorrect configuration, and our previous stack of
NGINX+Certbot was able to ignore this and serve without HTTPS. This PR
brings that behaviour back.
## Test performed
Have Appsmith running on an EC2 instance, and a domain `correct.com`
with an A-record pointed to this EC2 instance.
In the instance, we run Appsmith with `APPSMITH_CUSTOM_DOMAIN` set to
`wrong.com`. Caddy will obviously fail to provision the cert, and so we
expect it to accept connections on just HTTP.
So hitting `curl -i http://correct.com` produced a 200 with the HTML
response, and not a 308 with a redirect. Before the changes from this
PR, the same curl command produced a 308 with a redirect to
`https://correct.com`, which fails with a certificate error.
Next up, we run Appsmith with `APPSMITH_CUSTOM_DOMAIN` set to
`correct.com`. Caddy will succeed in provisioning a cert, and so we
expect HTTP URLs to be redirected to HTTPS.
So hitting `curl -i http://correct.com` produces a 308 redirect to
`http://correct.com` which then works fine, since Caddy now has the cert
for the domain.
We're setting the default value for `APPSMITH_ALLOWED_FRAME_ANCESTORS`
before we initialize env variables from `docker.env`. This make the
default value take a higher precedence over the value configured in
`docker.env`. And since the value in `docker.env` is the one configured
from Admin Settings, it feels like the value configured from the UI is
being ignored.
This fixes the problem by moving the check for this env variable to
_inside_ the reconfigure script, and so doesn't affect any env
variables.
Fixes#29114
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Performance Improvements**
- Enhanced logging capabilities to include memory footprint and context
details for better performance monitoring.
- **Configuration Updates**
- Increased the number of log file backups from 2 to 10, allowing for
more historical log retention.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Optimized auto-healing script by removing an unnecessary 60-second
delay.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Introduced auto-healing functionality to automatically restart
unresponsive backend services.
- Added SSL configuration support for custom domains.
- **Chores**
- Implemented periodic backend service status checks.
- Enhanced startup scripts to support new auto-healing feature based on
environment configuration.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
I think the route precedence in Caddy is different when using `handle`
directive, vs when directly using the `error` directive.
This is causing the file `handle {` route, which is a catch-all route is
handling `/static/*` requests that don't have a corresponding file. This
handler however, doesn't respond with 404 status, it responds with 200
status for missing files, and render the `index.html` for our SPA
behaviour.
Now, the CDN we have on release.app.appsmith.com caches responses from
upstream when the status is 200. If it is 404, it won't cache and retry
next time. This is why it's essential that we respond with 404 for files
that don't exist, irrespective of the content of the response.
When the container is starting up, Caddy doesn't have all the
information yet, and may have responded with not-found for one of the
assets. But since this went out with 200 status, our CDN cached it, and
once the file _was_ available with Caddy, the CDN wouldn't retry ever.
This fix will ensure we get 404 status code for requests to `/static/*`
that point to files that don't exist.
Fixes#29114
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Enhanced logging capabilities for better performance insights.
- **Improvements**
- Increased the number of log file backups to ensure more historical
data is preserved.
- **Documentation**
- Updated internal documentation to reflect new logging and performance
monitoring features.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This PR replaces NGINX and Certbot with Caddy.
1. Auto-HTTPS when custom domain is set, is handled by Caddy.
2. If past certs exist, that were provisioned by Certbot in older
Appsmith versions, we configure Caddy to make use of them. But this only
applies if the certs aren't already expired. If they're expired, point 1
applies.
3. If custom certs are provided in `ssl` folder, Caddy will be
configured to use them.
4. Incoming `Forwarded` header is not passed to any reverse proxies. So
redirect URL is correctly computed on Google Cloud Run.
5. All other route configurations are exactly as they are in NGINX
today.
Caddy configuration file is generated in the `caddy-reconfigure.mjs`
script, which will also reload Caddy with the new configuration.
## Description
This PR fixes the experience of Templates forking in self hosted
instances. And also for to Set up a process to keep the embedded DB up
to date with template db schemas.
We have removed the redirection of mockdb end point used in templates
App when forked in self hosted instance from localhost/internal postgres
db.
This also has a migration which is to make sure that none of existing
apps using the internal postgres does not break due to the removal of
redirection. The migration will make sure that existing self hosted
instances using the posgress db and has a datasource with mockdb end
point will be replaces with localhost.
#### PR fixes following issue(s)
Fixes https://github.com/appsmithorg/appsmith/issues/28924
#### Type of change
- Bug fix (non-breaking change which fixes an issue)
## Testing
#### How Has This Been Tested?
- [ ] Manual
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag
#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
---------
Co-authored-by: Shrikant Sharat Kandula <shrikant@appsmith.com>
Fix issue with alias names clashing in `keytool -import` command, when
there's more than one cert file in the `ca-certs` folder.
The fix is to explicitly set the alias for each `keytool -import` run,
to the file itself, so clashes don't happen.
Running an Appsmith as a non-root user:
```sh
docker run --name appsmith --user 70:70
```
The `70:70` figures are the UID and GID respectively. It can mostly be
any number, safe to user figures are 70 to 79, or anything above 200 and
below 65000. The important bit, is that it shouldn't change on restart
or manual updates etc.
No product functionality should be affected when running as a non-root
user.
The current latest release at
https://github.com/adoptium/temurin17-binaries/releases/tag/jdk-17.0.9%2B9.1,
doesn't include binaries for Linux, and so the Java download step fails
in our base Docker image build. This PR fixes that.
Sidenote, had we not had this base image and application image
separation, this bug would've blocked all our CI pipelines and the whole
team. 🙂
The layers in the Dockerfile that depend on downloading large files from
external sources, doesn't have to run every day, or at every PR. We
tried using Docker's caching configuration, but it's not as reliable as
we'd have liked.
A separate base image lends us much more control over the how long we
cache the downloaded files and how often we redo this.
This PR only _adds_ the base image. It doesn't change anything in the
build of the existing Docker image. That'll happen once we have the base
images for `release` and `master` already present on DockerHub.
