Supporting Custom CA without upstream changes - draft PR#308
Draft
anurag4DSB wants to merge 21 commits intomainfrom
Draft
Supporting Custom CA without upstream changes - draft PR#308anurag4DSB wants to merge 21 commits intomainfrom
anurag4DSB wants to merge 21 commits intomainfrom
Conversation
Implement TLS configuration for mounter pods to support HTTPS S3 endpoints with custom CA certificates. Uses an Alpine initContainer to install CA certificates into the system trust store, which is then shared with the main mount-s3 container via emptyDir volume. Key changes: - Add TLSConfig struct for CA certificate configuration - Add configureTLS() method to create volumes and initContainer - InitContainer installs ca-certificates package and runs update-ca-certificates to properly install custom CA - Add TLS environment variables to envprovider allowlist - Pass TLS config from controller main to mounter pod creator This approach is necessary because mount-s3's s2n-tls library only reads from /etc/ssl/certs/ and does not support AWS_CA_BUNDLE.
Add TLS configuration options to the Helm chart values and templates for custom CA certificate support in both controller and node pods. Chart values (values.yaml): - tls.caCertSecret: Name of Secret containing CA certificate - tls.initImage: Alpine image for cert installation initContainer - tls.initImagePullPolicy: Pull policy for init image - tls.initResources: Resource requests/limits for initContainer Controller template changes: - Mount CA certificate Secret to /etc/ssl/custom-ca - Set AWS_CA_BUNDLE environment variable when configured Node template changes: - Pass TLS configuration via environment variables to node pods - TLS_CA_CERT_SECRET_NAME and TLS_INIT_* environment variables
Add comprehensive documentation for configuring custom CA certificates when using HTTPS S3 endpoints with self-signed or internal CA certs. New documentation: - tls-configuration.md: Complete guide with examples, verification steps, and troubleshooting for TLS configuration - Updated helm-chart-configuration-reference.md with TLS options - Added navigation link in mkdocs.yml and index.md Documentation covers: - Creating Kubernetes Secrets with CA certificates - Helm chart configuration options - Verification steps for controller and mounter pods - Troubleshooting common TLS issues
Add scripts and configuration for testing the CSI driver with TLS-enabled S3 endpoints using self-signed certificates. New files: - generate-test-certs.sh: Script to generate CA and server certs for testing with OpenSSL Modified files: - docker-compose.yml: Add s3-tls profile for cloudserver with TLS - install.sh: Support TLS configuration via environment variables - run.sh: Pass TLS settings to test installation The s3-tls service generates self-signed certificates at startup and configures cloudserver to use HTTPS on port 8443.
Add GitHub Actions workflow job and Makefile targets for running end-to-end tests with TLS-enabled S3 endpoints. Workflow changes (e2e-tests.yaml): - Add e2e-tls job that runs tests against cloudserver with HTTPS - Deploy s3-tls service via docker-compose profile - Create CA certificate Secret from generated certs - Pass TLS configuration to Helm installation Makefile changes: - Add e2e-tls target for running TLS-specific tests - Add csi-install-tls for installing with TLS configuration
33960c8 to
640206f
Compare
Add comprehensive glossary entries for new concepts introduced in TLS configuration and mounter pod architecture documentation. New acronyms: - CA, FUSE, PEM, SSL, TLS New Kubernetes terms: - EmptyDir volume type New CSI Driver Architecture section: - Controller Service, Node Service, Mounter Pod - mount-s3, MountpointS3PodAttachment - Workload Pod New TLS and Security Terms section: - aws-lc, s2n-tls, X.509 - CA Bundle, Certificate Validation, Custom CA - Self-signed Certificate, System Trust Store - TLS Handshake, Trust Store New Operational Terms: - Certificate Rotation, Orchestration - Pod Lifecycle, Reconciliation Storage Terms: - Dynamic Provisioning (added definition)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 2 files with indirect coverage changes @@ Coverage Diff @@
## main #308 +/- ##
==========================================
+ Coverage 71.97% 72.82% +0.84%
==========================================
Files 47 47
Lines 2669 2756 +87
==========================================
+ Hits 1921 2007 +86
Misses 621 621
- Partials 127 128 +1 |
The CI was failing because it tried to start the cloudserver container before generating the required TLS certificates. The container expects certificates to be mounted via volume, not generated internally. Changes: - Renamed step to 'Generate TLS certificates' - Call generate-test-certs.sh BEFORE starting container - Use correct hostname: s3.scality.com (matches CoreDNS config) - Set ENDPOINT environment variable for cloudserver - Removed incorrect 'Wait for CA certificate and copy' step (was trying to copy certs FROM container after startup) The previous flow incorrectly assumed cloudserver would generate certificates internally. The correct flow is: 1. Generate certs with generate-test-certs.sh 2. Start cloudserver with certs mounted via volume 3. Cloudserver uses the provided certificates for TLS
Add comprehensive unit tests for the GetTLSConfig() function that reads TLS configuration from environment variables. Test coverage: - Returns nil when required TLS_CA_CERT_SECRET is not set - Returns nil when TLS_CA_CERT_SECRET is empty - Returns populated config with all fields set - Returns partial config with only some optional fields - Handles edge cases (limits without requests, etc.) The tests follow existing patterns in provider_test.go using table-driven tests with t.Setenv() for environment variable management.
Add container status check and log output before waiting for port to help diagnose why cloudserver with TLS isn't starting properly. This will show: - Container status (running/exited/restarting) - Last 50 lines of container logs - Any startup errors or configuration issues
Cloudserver uses port 8443 when SSL is enabled, not port 8000. Changes: - Wait for port 8443 instead of 8000 after starting TLS service - Update curl verification to use port 8443 - Update S3_ENDPOINT_URL for e2e tests to https://s3.scality.com:8443 This matches the standard behavior where: - HTTP uses port 8000 - HTTPS uses port 8443
The cloudserver entrypoint script requires openssl when SSL is enabled, but the container image doesn't include it by default. Changes: - Install openssl using apt-get before starting cloudserver - Suppress apt-get output to keep logs clean - This allows the entrypoint script to process SSL certificates properly Error fixed: /usr/src/app/docker-entrypoint.sh: line 54: openssl: command not found
The entrypoint script runs before the command, so openssl must be installed before the entrypoint executes. Changes: - Override entrypoint to /bin/sh - Command now: install openssl, then exec original entrypoint - Using 'exec' ensures the entrypoint replaces the shell process This fixes the error: /usr/src/app/docker-entrypoint.sh: line 54: openssl: command not found
Remove 'yarn start' arguments when calling docker-entrypoint.sh to allow the entrypoint script to properly process SSL environment variables and start the server on the correct port (8443). The entrypoint script is designed to handle the SSL configuration and server startup internally when SSL=true is set.
Add certFilePaths section to cloudserver-config.json to specify custom SSL certificate paths. CloudServer requires certificate paths in config.json, not just environment variables. Also restore 'yarn start' argument to docker-entrypoint.sh call as the entrypoint script expects it to start the server. References: - https://s3-server.readthedocs.io/en/latest/DOCKER.html
Create separate cloudserver-config-tls.json with port 8443 for HTTPS, instead of using port 8000. The docker-entrypoint.sh does not provide a way to override the port via environment variables, so we need a dedicated config file for TLS. The s3-tls service now uses: - cloudserver-config-tls.json (port 8443) - certFilePaths pointing to /certs/ The regular s3 service continues using: - cloudserver-config.json (port 8000, HTTP)
Remove unnecessary single quotes around ADDITIONAL_HELM_ARGS in Makefile that were causing shell parsing errors. The value is already properly quoted when passed from the workflow, and the extra quotes were creating nested quoting issues. This fixes the 'unexpected EOF while looking for matching quote' error in the e2e-all target.
Add double quotes around ADDITIONAL_HELM_ARGS to keep it as a single token when passed to the shell script. The install.sh script expects --additional-helm-args to be followed by a single argument containing all Helm args. Without quotes: --additional-helm-args --set tls.caCertSecret=... splits into multiple tokens, causing 'Unknown parameter' error With quotes: --additional-helm-args "--set tls.caCertSecret=..." passes as single token correctly
Split e2e-all into separate install and test phases to allow Helm to create the mount-s3 namespace with proper annotations before creating the CA certificate secret. Workflow order now: 1. Delete mount-s3 namespace (if exists from previous runs) 2. Helm install creates mount-s3 with Helm annotations 3. Create CA cert secret in Helm-managed mount-s3 namespace 4. Run E2E tests This fixes the 'invalid ownership metadata' error where manually created namespaces couldn't be imported into Helm releases.
Change from escaped double quotes \" to single quotes ' for ADDITIONAL_HELM_ARGS to prevent the quotes from being included as literal characters in the shell argument. Escaped double quotes were being passed as literal \" characters causing 'unexpected EOF while looking for matching quote' errors. Single quotes properly quote the entire argument value without escaping issues.
Avoid shell quoting issues by passing ADDITIONAL_HELM_ARGS as an environment variable (E2E_ADDITIONAL_HELM_ARGS) instead of trying to build it into the command-line arguments string. This eliminates the multi-layer quoting problem where we couldn't properly pass multi-word arguments through unquoted variable expansion in the Makefile. The install.sh script now reads E2E_ADDITIONAL_HELM_ARGS from the environment if not provided as a command-line argument.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supporting Custom CA without upstream changes - draft PR