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

fix: secrets not archived in workspace crates #785

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ cargo run --manifest-path ../../../Cargo.toml --bin cargo-shuttle -- logs
The steps outlined above starts all the services used by shuttle locally (ie. both `gateway` and `deployer`). However, sometimes you will want to quickly test changes to `deployer` only. To do this replace `make up` with the following:

```bash
# first generate the local docker-compose file
make docker-compose.rendered.yml

# then run it
docker compose -f docker-compose.rendered.yml up provisioner
```

Expand Down Expand Up @@ -178,13 +182,14 @@ config (which will be a file named `config.toml` in a directory named `shuttle`
echo "api_key = '<jwt>'" > ~/.config/shuttle/config.toml
```

> Note: if you have [`jq`](https://github.com/stedolan/jq/wiki/Installation) installed you can combine the two
>above commands into the following:
>```bash
>curl -s -H "Authorization: Bearer test-key" localhost:8008/auth/key \
> | jq -r '.token' \
> | read token; echo "api_key='$token'" > ~/.config/shuttle/config.toml
>```
> Note: The JWT will expire in 15 minutes, at which point you need to run the commands again.
> If you have [`jq`](https://github.com/stedolan/jq/wiki/Installation) installed you can combine
> the two above commands into the following:
> ```bash
> curl -s -H "Authorization: Bearer test-key" localhost:8008/auth/key \
> | jq -r '.token' \
> | read token; echo "api_key='$token'" > ~/.config/shuttle/config.toml
> ```
Finally we need to comment out the admin layer in the deployer handlers. So in `deployer/handlers/mod.rs`,
in the `make_router` function comment out this line: `.layer(AdminSecretLayer::new(admin_secret))`.
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ deploy: docker-compose.yml
test:
cd e2e; POSTGRES_PASSWORD=$(POSTGRES_PASSWORD) APPS_FQDN=$(APPS_FQDN) cargo test $(CARGO_TEST_FLAGS) -- --nocapture

docker-compose.rendered.yml: docker-compose.yml docker-compose.dev.yml
$(DOCKER_COMPOSE_ENV) $(DOCKER_COMPOSE) -f docker-compose.yml -f docker-compose.dev.yml $(DOCKER_COMPOSE_CONFIG_FLAGS) -p $(STACK) config > $@

# Start the containers locally. This does not start panamax by default,
# to start panamax locally run this command with an override for the profiles:
# `make COMPOSE_PROFILES=panamax up`
Expand Down
14 changes: 12 additions & 2 deletions cargo-shuttle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,8 +853,18 @@ impl Shuttle {
{
let dir_entry = dir_entry.context("get directory entry")?;

// It's not possible to add a directory to an archive
if dir_entry.file_type().context("get file type")?.is_dir() {
let secrets_path = dir_entry.path().join("Secrets.toml");

// Make sure to add any `Secrets.toml` files in the subdirectories.
if secrets_path.exists() {
Copy link
Contributor

@iulianbarbu iulianbarbu Apr 11, 2023

Choose a reason for hiding this comment

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

Should we check this path points to a file and not a directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure we need to since we join Secrets.toml to it. I think it is technically still a valid directory name, but in the event that anyone names a dir in their project Secrets.toml I think it will just be added as an empty dir to the archive.

let path = secrets_path
.strip_prefix(base_directory)
.context("strip the base of the archive entry")?;
tar.append_path_with_name(secrets_path.clone(), path)?;
}

// It's not possible to add a directory to an archive
continue;
}

Expand All @@ -867,7 +877,7 @@ impl Shuttle {
.context("archive entry")?;
}

// Make sure to add any `Secrets.toml` files
// Make sure to add any `Secrets.toml` files in the root of the workspace.
let secrets_path = self.ctx.working_directory().join("Secrets.toml");
if secrets_path.exists() {
tar.append_path_with_name(secrets_path, Path::new("shuttle").join("Secrets.toml"))?;
Expand Down
1 change: 0 additions & 1 deletion cargo-shuttle/src/logger.rs

This file was deleted.

13 changes: 10 additions & 3 deletions deployer/src/deployment/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,6 @@ impl Queued {

extract_tar_gz_data(self.data.as_slice(), &project_path).await?;

let secrets = get_secrets(&project_path).await?;
set_secrets(secrets, &self.service_id, secret_recorder).await?;

info!("Building deployment");

let (tx, rx): (crossbeam_channel::Sender<Message>, _) = crossbeam_channel::bounded(0);
Expand Down Expand Up @@ -213,8 +210,18 @@ impl Queued {
});

let project_path = project_path.canonicalize()?;

// Currently returns the first found shuttle service in a given workspace.
let runtime = build_deployment(&project_path, tx.clone()).await?;

// Get the Secrets.toml from the shuttle service in the workspace.
let secrets = get_secrets(&runtime.working_directory).await?;

// Set the secrets from the service, ignoring any Secrets.toml if it is in the root of the workspace.
// TODO: refactor this when we support starting multiple services. Do we want to set secrets in the
// workspace root?
set_secrets(secrets, &self.service_id, secret_recorder).await?;

if self.will_run_tests {
info!(
build_line = "Running tests before starting up",
Expand Down