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 checking and logging of envVars in deployApp() #994

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
26 changes: 20 additions & 6 deletions R/deployApp.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,12 @@
#' on second and subsequent deploys, the title will be unchanged.
#' @param envVars A character vector giving the names of environment variables
#' whose values should be synchronised with the server (currently supported by
#' Connect only). The values of the environment variables are sent over an
#' encrypted connection and are not stored in the bundle, making this a safe
#' way to send private data to Connect.
#' Connect only). The values of sensitive environment variables should be set
#' in the current session via an `.Renviron` file or with the help of a
#' credential store like
#' [keyring](https://r-lib.github.io/keyring/index.html). Values are sent over
#' an encrypted connection and are not stored in the bundle, making this a
#' safe way to send private data to Connect.
#'
#' The names (not values) are stored in the deployment record so that future
#' deployments will automatically update their values. Other environment
Expand Down Expand Up @@ -258,6 +261,15 @@ deployApp <- function(appDir = getwd(),
}
}

check_character(envVars, allow_null = TRUE)
if (!is.null(envVars) && !is.null(names(envVars))) {
cli::cli_abort(c(
"{.arg envVars} must be a character vector containing only environment variable {.strong names}.",
"i" = "Set environment variables with `Sys.setenv() or an `.Renviron` file.",
"i" = "Use {.fn unname} to remove the names from the vector passed to {.arg envVars}."
))
}

if (!is.null(appSourceDoc)) {
# Used by IDE so can't deprecate
recordDir <- appSourceDoc
Expand Down Expand Up @@ -360,12 +372,14 @@ deployApp <- function(appDir = getwd(),
stop("Posit Connect does not support deploying without uploading. ",
"Specify upload=TRUE to upload and re-deploy your application.")
}
if (!isConnectServer(target$server) && length(envVars) > 1) {
cli::cli_abort("{.arg envVars} only supported for Posit Connect servers")
}

accountDetails <- accountInfo(target$account, target$server)
client <- clientForAccount(accountDetails)

if (length(envVars) > 0 && !"setEnvVars" %in% names(client)) {
cli::cli_abort("{target$server} does not support setting {.arg envVars}")
}

if (verbose) {
showCookies(serverInfo(accountDetails$server)$url)
}
Expand Down
9 changes: 6 additions & 3 deletions man/deployApp.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions tests/testthat/_snaps/deployApp.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,13 @@
2: Delete existing deployment & create a new app
Selection: 2

# deployApp() errors if envVars is given a named vector

Code
deployApp(local_temp_app(), envVars = c(FLAG = "true"))
Condition
Error in `deployApp()`:
! `envVars` must be a character vector containing only environment variable names.
i Set environment variables with `Sys.setenv() or an `.Renviron` file.
i Use `unname()` to remove the names from the vector passed to `envVars`.

8 changes: 8 additions & 0 deletions tests/testthat/test-deployApp.R
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,11 @@ test_that("applicationDeleted() errors or prompts as needed", {
expect_snapshot(. <- applicationDeleted(client, target, app))
expect_length(dir(app, recursive = TRUE), 0)
})

# envvars -----------------------------------------------------------------

test_that("deployApp() errors if envVars is given a named vector", {
expect_snapshot(error = TRUE, {
deployApp(local_temp_app(), envVars = c("FLAG" = "true"))
})
})
Loading