feat: address reviews, add more edge cases to test

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

extensions/env-injector/src/main.rs

@@ -43,20 +43,51 @@ async fn snapdapi_req() -> Result<serde_json::Value, Box<dyn Error + Send + Sync
     Ok(serde_json::from_slice(&body)?)
 }
 
+fn process_env(env: &serde_json::Value) -> HashMap<String, String> {
+    let obj = env.as_object()
+        .ok_or("Expected an object (JSON input)").unwrap();
+    let mut map = HashMap::new();
+
+    for (k, v) in obj {
+        if v.is_object() || v.is_array() {
+            eprintln!(
+                "ERROR: Invalid value detected.\n\
+                    Key: {}\n\
+                    Reason: Environment variable names must not contain dots.\n\
+                    Action: Skipped\n",
+                v
+            );
+            continue;
+        }
+
+        let key = k.to_uppercase().replace("-", "_");
+        let value = match v {
+            serde_json::Value::String(s) => s.clone(),
+            serde_json::Value::Number(n) => n.to_string(),
+            serde_json::Value::Bool(b) => b.to_string(),
+            _ => {
+                eprintln!(
+                    "ERROR: Invalid type for environment variable value detected.\n\
+                        val: {}\n\
+                        Reason: It must be a string, number or boolean\n\
+                        Action: Skipped\n",
+                    k
+                );
+                continue;
+            },
+        };
+        map.insert(key, value);
+    }
+    map
+}
+
+
+
+
 fn set_env_vars(app: &str, json: &serde_json::Value) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
     let stdout_str = json["result"]["stdout"].as_str().ok_or("Invalid stdout")?;
     let stdout_json: serde_json::Value = serde_json::from_str(stdout_str)?;
 
-    fn process_env(env: &serde_json::Value) -> HashMap<String, String> {
-        env.as_object()
-            .unwrap()
-            .iter()
-            .map(|(k, v)| {
-                let key = k.to_uppercase().replace("-", "_");
-                (key, v.to_string())
-            })
-            .collect()
-    }
 
     if let Some(global_env) = stdout_json["env"].as_object() {
         for (key, value) in process_env(&serde_json::Value::Object(global_env.clone())) {

snapcraft/extensions/env_injector.py

@@ -46,6 +46,9 @@ class EnvInjectorExtension(Extension):
 
     .. code-block:: shell
         sudo snap set <snap-name> env-file=<path-to-env-file>
+    - To set environment file for a specific app:
+    .. code-block:: shell
+        sudo snap set <snap-name> apps.<app-name>.envfile=<path-to-env-file>
 
     """
 
@@ -97,7 +100,6 @@ def get_parts_snippet(self) -> Dict[str, Any]:
                     "rustup",
                 ],
                 "build-packages": [
-                    "musl-tools",  # for static linking
                     "upx-ucl",  # for binary compression
                 ],
                 "override-build": f"""

tests/spread/extensions/env-injector/task.yaml

@@ -41,13 +41,28 @@ execute: |
   snap install "${SNAP}"_1.0_*.snap --dangerous
 
   # Create envfile
-  echo 'HELLO_WORLD="Hello, World"' >> envfile.env
+  echo 'HELLO_WORLD=Hello World' >> envfile.env
 
-  # Set env vars: Global, App specific, and envfile
-  snap set env-injector-hello env.hello="Hello"
-  snap set env-injector-hello apps.hello.env.world="World"
-  snap set env-injector-hello envfile="${SNAP_DIR}"/enfile.env
+  # Set env vars: Global
+  snap set env-injector-hello env.global="World"
+
+  # Set env vars: specific to each app
+  snap set env-injector-hello apps.hello1.env.hello="Hello"
+  snap set env-injector-hello apps.hello2.env.specific="City"
+
+  # To Check if key with dot will be rejected 
+  snap set env-injector-hello env.word.dot="wrong"
+
+  # To test order of env vars
+  echo 'ORDER="From envfile"' >> envfile.env
+  snap set env-injector-hello apps.hello1.env.order="from app-specific"
+  snap set env-injector-hello envfile="${SNAP_DIR}"/envfile.env
 
   # Run the hello command
-  env-injector-hello.hello
+  # Those will fail if the env vars are not set as expected
+  env-injector-hello.hello1
+  env-injector-hello.hello2
 
+  # Test if the alias it's rewrite
+  snap set env-injector-hello apps.hello-demo.env.specific="City"
+  env-injector-hello.hello-demo

tests/spread/extensions/snaps/env-injector-hello/CMakeLists.txt

@@ -1,4 +1,10 @@
 cmake_minimum_required(VERSION 3.5)
 project(hello C)
+
+# Add executable for hello
 add_executable(hello hello.c)
 install(TARGETS hello RUNTIME DESTINATION bin)
+
+# Add executable for hello2
+add_executable(hello2 hello2.c)
+install(TARGETS hello2 RUNTIME DESTINATION bin)

tests/spread/extensions/snaps/env-injector-hello/hello.c

@@ -4,34 +4,66 @@
 
 int main(int argc, char const *argv[]) {
     const char *envs[] = {
-        "HELLO",        // This is set globally
-        "WORLD",        // This is set for the app
-        "HELLO_WORLD",  // This is set from env file
+        "GLOBAL",       // This is set globally
+        "HELLO",        // This is set for the app
+        "HELLO_WORLD",  // This is set from global env file
     };
 
     const char *expected[] = {
-        "Hello",
         "World",
+        "Hello",
         "Hello World",
     };
 
     int num_envs = sizeof(envs) / sizeof(envs[0]);
 
+    // Check Global, App specific and global env file
     for (int i = 0; i < num_envs; ++i) {
         const char *env = getenv(envs[i]);
 
         if (env == NULL) {
             fprintf(stderr, "\n[ERROR] Env. variable %s is not set.\n", envs[i]);
-            return 1;
+            exit(1);
         }
 
         if (strcmp(env, expected[i]) != 0) {
             fprintf(stderr, "\n[ERROR] Env. variable %s isn't set to the expected value.\n", envs[i]);
             fprintf(stderr, "Expected: %s\n", expected[i]);
             fprintf(stderr, "Got: %s\n", env);
-            return 1;
+            exit(1);
         }
     }
 
+    // Check that it's not possible to access other app ENVs 
+    const char *env_specific = getenv("SPECIFIC");
+    if (env_specific != NULL) {
+        fprintf(stderr, "\n[ERROR] Env. variable SPECIFIC is accessible from the app.\n");
+        fprintf(stderr, "Expected: NULL\n");
+        fprintf(stderr, "Got %s\n", env_specific);
+        exit(1);
+    }
+
+    // Check if key with dot was rejected 
+    const char *env_dot = getenv("DOT");
+    if (env_dot != NULL) {
+        fprintf(stderr, "\n[ERROR] Received Env. variable DOT with wrong key subset.\n");
+        fprintf(stderr, "Expected: NULL\n");
+        fprintf(stderr, "Got %s\n", env_dot);
+        exit(1);
+    }
+
+    // Precedence check: Testing if the order of the envs is correct
+    const char *env_order = getenv("ORDER");
+
+    if (env_order == NULL){
+        fprintf(stderr, "\n[ERROR] Env. variable ORDER is not set.\n");
+        exit(1);
+    }
+
+    if (strcmp(env_order, "from app-specific") != 0) {
+        fprintf(stderr, "\n[ERROR] Precedence error: app-specific envs should override global envs.\n");
+        exit(1);
+    }
+
     return 0;
 }

tests/spread/extensions/snaps/env-injector-hello/hello2.c

@@ -0,0 +1,46 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char const *argv[]) {
+    const char *envs[] = {
+        "GLOBAL",       // This is set globally
+        "SPECIFIC",     // This is set for the app only
+        "HELLO_WORLD",  // This is set from global env file
+    };
+
+    const char *expected[] = {
+        "World",
+        "City",
+        "Hello World",
+    };
+
+    int num_envs = sizeof(envs) / sizeof(envs[0]);
+
+    for (int i = 0; i < num_envs; ++i) {
+        const char *env = getenv(envs[i]);
+
+        if (env == NULL) {
+            fprintf(stderr, "\n[ERROR] Env. variable %s is not set.\n", envs[i]);
+            exit(1);
+        }
+
+        if (strcmp(env, expected[i]) != 0) {
+            fprintf(stderr, "\n[ERROR] Env. variable %s isn't set to the expected value.\n", envs[i]);
+            fprintf(stderr, "Expected: %s\n", expected[i]);
+            fprintf(stderr, "Got: %s\n", env);
+            exit(1);
+        }
+    }
+
+    // Check that it's not possible to access other app ENVs 
+    const char *env_hello = getenv("HELLO");
+    if (env_hello != NULL) {
+        fprintf(stderr, "\n[ERROR] Env. variable SPECIFIC is accessible from the app.\n");
+        fprintf(stderr, "Expected: NULL\n");
+        fprintf(stderr, "Got %s\n", env_hello);
+        exit(1);
+    }
+
+    return 0;
+}

tests/spread/extensions/snaps/env-injector-hello/snap/snapcraft.yaml

@@ -8,8 +8,18 @@ base: core24
 confinement: strict
 
 apps:
-  env-hello:
+  hello1:
     command: usr/local/bin/hello
+    extensions: [ env-injector ]  
+
+  hello2:
+    command: usr/local/bin/hello2
+    extensions: [ env-injector ]
+
+  hello-demo:
+    command: usr/local/bin/hello2
+    environment:
+      app_alias: hello-demo
     extensions: [ env-injector ]
 
 parts: