Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Feature] Set environment variables for an app #382

Closed
ggwadera opened this issue Sep 7, 2020 · 21 comments · Fixed by #383
Closed

[Feature] Set environment variables for an app #382

ggwadera opened this issue Sep 7, 2020 · 21 comments · Fixed by #383
Assignees
Labels

Comments

@ggwadera
Copy link
Collaborator

ggwadera commented Sep 7, 2020

Currently there is no way for an user to set environment variables for their apps if they need to. This feature would enable a new configuration option for the apps, so an user can add, edit, or remove environment variables for an app.

Goals

  • Add new button in the client frontend that an user can click and then add, edit, or remove environment variables for their app.
  • Handle the environment variables modifications through a new API endpoint in the server.

Design

The environment variables are stored inside the docker container configuration files, and we can get those variables by inspecting a container with the docker API, so there's no need to store them somewhere else.

To modify the environment variables for a existing container, we have to recreate the container with the new variables.

In summary, the steps for this feature are:

  1. User presses the Manage button for the domain the user wants to edit.

image

  1. The user is redirected to a new page (/manage/${fullDomain}) where the user can manage the environment variables (add, modify, delete).

image

  • The Add button creates a new input form at the end of the list.
  • The Remove button will remove that environment variable from the list.
  • The Submit button will send a POST request to the API with the environment variables on the list.
  • The NAME input is automatically converted to uppercase and validated to contain only letters, numbers and/or underscore.

image

  1. The client queries the server for the container existing environment variables.

    1. Server queries docker to inspect the container and get the container info back.

    2. Server filters out the default environment variables like NODE_ENV and PORT, then returns the filtered variables.

    • The app environment variables are stored inside the container config file generated by Docker (API reference). For example:
    "Env": [
      "NODE_ENV=production",
      "PORT=3000",
      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
      "NODE_VERSION=14.9.0",
      "YARN_VERSION=1.22.5"
    ],
  2. User presses the submit button to send the new environment variables to the server.

  3. If the container is running, warns the user to confirm that the app will be restarted and asks if it's OK.

  4. The server stops and removes the existing container.

  5. The server recreates the container with the new environment variables and starts the container.

    • Example Docker configuration with a new environment variable
    "Env": [
      "NODE_ENV=production",
      "PORT=3000",
      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
      "NODE_VERSION=14.9.0",
      "YARN_VERSION=1.22.5",
      "MY_EXAMPLE_VAR=my_example_var_value"
    ],

API Spec

GET: /api/mappings/${fullDomain}/environment

This endpoint will query the Docker API to inspect the container configurations, extract the current environment variables, and then filter out the default variables (NODE_ENV, PORT, PATH, NODE_VERSION, YARN_VERSION), returning the non-default variables with name and value.

Response sample (success):

{
  variables: {
    NAME1: "value1",
    NAME2: "value2"
  }
}

Response sample (failure):

{
  message: "error message"
}

PUT: /api/mappings/${fullDomain}/environment

This will save the new environment variables for the app. To do this, the container needs to be recreated with the new environment variables appended in the container configuration options (like restarting an app to pass a new env variable). After recreating, the container is started.

As we are using the same ID from the Docker container as the domain mapping ID, this will also update the mapping ID to the new container ID and update the data.db file.

Request body sample:

{
  variables: {
    NAME1: "value1",
    NAME2: "value2"
  }
}

Response sample (success): status code 204, no body.

Response sample (failure):

{
  message: "error message"
}
@ggwadera ggwadera self-assigned this Sep 7, 2020
@songz
Copy link
Collaborator

songz commented Sep 8, 2020

step 3: /edit/${id} -> is id the domain name? or internal id of the domain?

@ggwadera
Copy link
Collaborator Author

ggwadera commented Sep 8, 2020

step 3: /edit/${id} -> is id the domain name? or internal id of the domain?

I'm using the internal id, which is the docker container id, but both can be used as docker can find a container by it's id or name(=full domain).

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

the UI is great, specifically I think Edit Domain should be Manage Domain. Edit suggests that you can change the url or port, which we don't support yet.

When you click on add or remove, could you define the API spec as well? What APIs would be needed, request body and response sample, and then a brief description of what the API would do internally.

@coltonehrman

This comment has been minimized.

@coltonehrman

This comment has been minimized.

@coltonehrman

This comment has been minimized.

@ggwadera
Copy link
Collaborator Author

ggwadera commented Sep 8, 2020

Question on updated ENV variables in a container, is it not possible to do this live in a running container?
Is the only way to kill the running container and re-run it?

Unfortunately Docker doesn't provide an direct way to do this.

There is a hacky approach where you modify the Docker container json config file, but then we have to restart the Docker daemon which will stop all the apps until it's restarted.

@coltonehrman

This comment has been minimized.

@ggwadera
Copy link
Collaborator Author

ggwadera commented Sep 8, 2020

I don't like the idea of having a domain inside a path /manage/${fullDomain} could we use some kind of ID instead? /manage/${id}

So this was something that I briefly discussed with @songz. I originally was going to use the ID for this, but he thought better to use the domain name to not reveal the internal IDs and to make it more readable at the same time.

@coltonehrman

This comment has been minimized.

@coltonehrman

This comment has been minimized.

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

I don't like the idea of having a domain inside a path /manage/${fullDomain} could we use some kind of ID instead? /manage/${id}

So this was something that I briefly discussed with @songz. I originally was going to use the ID for this, but he thought better to use the domain name to not reveal the internal IDs and to make it more readable at the same time.

Hmm... Idk haha
I guess if we can be confident that the ${fullDomain} will only ever appear like app.domain.com Maybe it is ok? It just looks weird imo.
We either have:
https://www.freedomains.dev/manage/sdj6wne2384ufhdh2
or
https://www.freedomains.dev/manage/app.domain.com

Maybe we can do a vote on it haha

@coltonehrman readable domains in the url is generally preferred. Imagine if twitter handle was ids used in the systems internally. Using id adds no value, why do you want it?

@coltonehrman

This comment has been minimized.

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

https://www.google.com/https://www.freedomains.dev/app

Would be: https://www.myProxy.com/www.freedomains.dev/ Since we only care about subdoain + domain, we can exclude port and path.

@coltonehrman

This comment has been minimized.

@coltonehrman

This comment has been minimized.

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

@ggwadera POST: /api/mappings/${fullDomain}/environment -> This should be a PUT right? Because you are creating if doesn't exist, and then overwriting the existing configs correct?

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

This will save the new environment variables for the app

Would the environment variables be saved into data.db? Could you add a snippet of how the ending data.db data would look like?

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

step 2 comes after step 3 right? page loads then query for env vars?

@songz
Copy link
Collaborator

songz commented Sep 8, 2020

If the container is running, warns the user to confirm that the app will be restarted a -> If the container is running, warns the user to confirm that the app will destroyed and recreated so all new files and data created within the container will be gone

@ggwadera
Copy link
Collaborator Author

ggwadera commented Sep 8, 2020

This will save the new environment variables for the app

Would the environment variables be saved into data.db? Could you add a snippet of how the ending data.db data would look like?

As Docker already stores the environment variables inside the container configuration, I thought we could use that instead of storing in another place. The only modification in data.db is for the domain mapping ID, which is generated again when the container is recreated.

I included an example of how the Docker configuration would look like with a new variable.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants