Fix checking and logging of envVars in deployApp()

[gadenbuie]

Two small fixes for the checking of envVars in deployApp():

1. First, the values of the envvars were being printed in to the logs, where we'd only want the names.

2. The check ensuring that envVars is only used for Posit Connect servers would not run if the user was only setting a single variable.

[gadenbuie]

Rather than checking here by enforcing that target$server points to a Connect server, you could instead check just a little bit further down after creating client. In that case, you could use a block like the following, and check that client implements the setEnvVars() psuedo-method.

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

[hadley]

I like that idea. Want to do it?

[gadenbuie]

No problem, done in f3dac5b

[hadley]

These are already just the names of the env vars?

[gadenbuie]

Sorry, I had a hard time following the flow for that property. What happened for me -- before I applied the other fix -- was that I tried to deploy an app with one env var to shinyapps.io. Something like envVars = c("FLAG" = "true"). This made it past the first check and failed here (because the shinyapps client doesn't implement $setEnvVars()) at which point this call was printing environment variables true.

I made this change first and then realized that the earlier check was failing, and in my probably semi-broken state envVars was a named vector. This may have been the result of a local rsconnect deploy record with a value that shouldn't have been written?

[hadley]

Yeah, maybe we need some earlier validation that envVars is an unnamed character vector?

[gadenbuie]

Ohhh, I read the documentation more carefully and realized that the environment variables should be set in the current session and the user should provide just the names, as in envVars = c("FLAG", "OTHER_FLAG").

I think my confusion stemmed from being familiar with connectapi, where there's an interface for setting environment variables during deployment that uses the FLAG = "value" pattern.

I updated the docs to give a little guidance about how those values should be set, and I added a check that the vector provided to envVars is unnamed. This feels a little aggressive but I think it's worth it, given the potential risk of leaking a credential if a user misunderstands how envVars should be used.

[hadley]

In case it's not clear, the interface looks like this is to prevent you from typing the values of env vars in executed code since that will be captured in .Rhistory.

[hadley]

I don't think it's good practice to recommend Sys.setenv() since that'll result in the value of the env var appearing in your .Rhistory. It might be good to say that explicitly here.

[gadenbuie]

Good point. I took another pass and rewrote this sentence:

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 sensitive environment variables should be set in the current session via an .Renviron file or with the help of a credential store like keyring. 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.