Changed engine runner not to use shell for subprocess (#704)

danyi1212 · web-flow · commit 93bb21e6ccd4 · 2024-11-18T12:46:52.000+02:00
diff --git a/cedar-agent b/cedar-agent
@@ -1 +1 @@
-Subproject commit 687efc59ecc732d1b98fc7789ab803abfc45b94c
+Subproject commit 7c93b3d9ffc0108857b43bdefab954a813964b8f
diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -116,6 +116,7 @@ COPY --from=opa-extractor /opal/opa ./opa
 
 # enable inline OPA
 ENV OPAL_INLINE_OPA_ENABLED=true
+ENV OPAL_INLINE_OPA_EXEC_PATH=/opal/opa
 # expose opa port
 EXPOSE 8181
 USER opal
@@ -134,6 +135,7 @@ COPY --from=cedar-builder /tmp/cedar-agent/target/*/cedar-agent /bin/cedar-agent
 # enable inline Cedar agent
 ENV OPAL_POLICY_STORE_TYPE=CEDAR
 ENV OPAL_INLINE_CEDAR_ENABLED=true
+ENV OPAL_INLINE_CEDAR_EXEC_PATH=/bin/cedar-agent
 ENV OPAL_INLINE_CEDAR_CONFIG='{"addr": "0.0.0.0:8180"}'
 ENV OPAL_POLICY_STORE_URL=http://localhost:8180
 # expose cedar port
diff --git a/docker/docker-compose-api-policy-source-example.yml b/docker/docker-compose-api-policy-source-example.yml
@@ -1,3 +1,5 @@
+name: opal-api-policy-source-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-example-cedar.yml b/docker/docker-compose-example-cedar.yml
@@ -1,3 +1,5 @@
+name: opal-cedar-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-example.yml b/docker/docker-compose-example.yml
@@ -1,3 +1,5 @@
+name: opal-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-git-webhook.yml b/docker/docker-compose-git-webhook.yml
@@ -1,3 +1,5 @@
+name: opal-git-webhook-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-scopes-example.yml b/docker/docker-compose-scopes-example.yml
@@ -1,3 +1,5 @@
+name: opal-scopes-example
+
 services:
   redis:
     image: redis
diff --git a/docker/docker-compose-with-callbacks.yml b/docker/docker-compose-with-callbacks.yml
@@ -1,3 +1,5 @@
+name: opal-callbacks-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-with-kafka-example.yml b/docker/docker-compose-with-kafka-example.yml
@@ -1,3 +1,4 @@
+name: opal-kafka-example
 
 services:
   # Based on: https://developer.confluent.io/quickstart/kafka-docker/
diff --git a/docker/docker-compose-with-oauth-initial.yml b/docker/docker-compose-with-oauth-initial.yml
@@ -1,3 +1,5 @@
+name: opal-oauth-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/docker/docker-compose-with-rate-limiting.yml b/docker/docker-compose-with-rate-limiting.yml
@@ -1,3 +1,5 @@
+name: opal-rate-limiting-example
+
 # This docker compose example shows how to configure OPAL's rate limiting feature
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
diff --git a/docker/docker-compose-with-security.yml b/docker/docker-compose-with-security.yml
@@ -1,3 +1,5 @@
+name: opal-security-example
+
 # this docker compose file is relying on external environment variables!
 # run it by running the script: ./run-example-with-security.sh
 services:
diff --git a/docker/docker-compose-with-statistics.yml b/docker/docker-compose-with-statistics.yml
@@ -1,3 +1,5 @@
+name: opal-statistics-example
+
 services:
   # When scaling the opal-server to multiple nodes and/or multiple workers, we use
   # a *broadcast* channel to sync between all the instances of opal-server.
diff --git a/documentation/docs/getting-started/configuration.mdx b/documentation/docs/getting-started/configuration.mdx
@@ -41,7 +41,6 @@ Please use this table as a reference.
 | AUTH_JWT_ALGORITHM                      | JWT algorithm. See possible values [here](https://pyjwt.readthedocs.io/en/stable/algorithms.html).                                                                              |         |
 | AUTH_JWT_AUDIENCE                       |                                                                                                                                                                                 |         |
 | AUTH_JWT_ISSUER                         |                                                                                                                                                                                 |                           |
-| INLINE_OPA_EXECUTABLE_PATH              | Path to the OPA executable. If not specified, defaults to "opa", assuming the OPA executable is in the system PATH.                                                             | /usr/local/bin/opa        |
 
 ## OPAL Server Configuration Variables
 
@@ -122,7 +121,7 @@ Please use this table as a reference.
 ## OPAL Client Configuration Variables
 
 | Variables                             | Description                                                                                                                                                                                                                                                                    | Example                              |
-| ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------ |
+| ------------------------------------- |--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| ------------------------------------ |
 | POLICY_STORE_TYPE                     |                                                                                                                                                                                                                                                                                |                                      |
 | POLICY_STORE_AUTH_TYPE                | The authentication method for connecting to the policy store. Possible values are `oauth` or `token`                                                                                                                                                                           |                                      |
 | POLICY_STORE_AUTH_TOKEN               | The authentication (bearer) token OPAL client will use to authenticate against the policy store (i.e: OPA agent).                                                                                                                                                              |                                      |
@@ -132,8 +131,13 @@ Please use this table as a reference.
 | POLICY_STORE_CONN_RETRY               | Retry options when connecting to the policy store (i.e. the agent that handles the policy, e.g. OPA).                                                                                                                                                                          |                                      |
 | POLICY_STORE_POLICY_PATHS_TO_IGNORE   | Which policy paths pushed to the client should be ignored. List of glob style paths, or paths without wildcards but ending with "/\*\*" indicating a parent path (ignoring all under it).                                                                                      |                                      |
 | INLINE_OPA_ENABLED                    | Whether or not OPAL should run OPA by itself in the same container.                                                                                                                                                                                                            |                                      |
+| INLINE_OPA_EXEC_PATH                  | The path to the OPA executable.                                                                                                                                                                                                                                                |                                      |
 | INLINE_OPA_CONFIG                     | If inline OPA is indeed enabled, the user can set the [server configuration options](https://docs.opal.ac/getting-started/running-opal/run-opal-client/opa-runner-parameters) that affects how OPA will start when running `opa run --server` inline.  Watch escaping quotes.  | \{"config_file":"/mnt/opa/config"\}  |
 | INLINE_OPA_LOG_FORMAT                 |                                                                                                                                                                                                                                                                                |                                      |
+| INLINE_CEDAR_ENABLED                  | Whether or not OPAL should run Cedar agent by itself in the same container.                                                                                                                                                                                                    |                                      |
+| INLINE_CEDAR_EXEC_PATH                | The path to the Cedar agent executable.                                                                                                                                                                                                                                        |                                      |
+| INLINE_CEDAR_CONFIG                   | If inline Cedar is indeed enabled, provide options for running the Cedar agent                                                                                                                                                                                                 |                                      |
+| INLINE_CEDAR_LOG_FORMAT               |                                                                                                                                                                                                                                                                                |                                      |
 | KEEP_ALIVE_INTERVAL                   |                                                                                                                                                                                                                                                                                |                                      |
 | OFFLINE_MODE_ENABLED                  | If set, opal client will try to load policy store from backup file and operate even if server is unreachable. Ignored if INLINE_OPA_ENABLED=False                                                                                                                              |                                      |
 | STORE_BACKUP_PATH                     | Path to backup policy store's data to                                                                                                                                                                                                                                          |                                      |
diff --git a/packages/opal-client/opal_client/config.py b/packages/opal-client/opal_client/config.py
@@ -134,6 +134,12 @@ def load_policy_store():
     # whether or not OPAL should run OPA by itself in the same container
     INLINE_OPA_ENABLED = confi.bool("INLINE_OPA_ENABLED", True)
 
+    INLINE_OPA_EXEC_PATH = confi.str(
+        "INLINE_OPA_EXEC_PATH",
+        None,
+        description="Path to the OPA executable. Defaults to searching for 'opa' binary in PATH if not specified.",
+    )
+
     # if inline OPA is indeed enabled, user can pass cli options
     # (configuration) that affects how OPA will run
     INLINE_OPA_CONFIG = confi.model(
@@ -147,17 +153,17 @@ def load_policy_store():
         "INLINE_OPA_LOG_FORMAT", EngineLogFormat, EngineLogFormat.NONE
     )
 
-    INLINE_OPA_EXECUTABLE_PATH = confi.str(
-        "INLINE_OPA_EXECUTABLE_PATH",
-        "opa",
-        description="Path to the OPA executable. Defaults to 'opa' if not specified.",
-    )
-
     # Cedar runner configuration (Cedar-engine can optionally be run by OPAL) ----------------
 
     # whether or not OPAL should run the Cedar agent by itself in the same container
     INLINE_CEDAR_ENABLED = confi.bool("INLINE_CEDAR_ENABLED", True)
 
+    INLINE_CEDAR_EXEC_PATH = confi.str(
+        "INLINE_CEDAR_EXEC_PATH",
+        None,
+        description="Path to the Cedar Agent executable. Defaults to searching for 'cedar-agent' binary in PATH if not specified.",
+    )
+
     # if inline Cedar is indeed enabled, user can pass cli options
     # (configuration) that affects how the agent will run
     INLINE_CEDAR_CONFIG = confi.model(
diff --git a/packages/opal-client/opal_client/engine/options.py b/packages/opal-client/opal_client/engine/options.py
@@ -1,8 +1,13 @@
+import re
 from enum import Enum
-from typing import Any, List, Optional
+from typing import Any, Iterable, List, Optional
 
 from pydantic import BaseModel, Field, validator
 
+HOST_ADDR_PATTERN = re.compile(
+    r"^(?P<addr>(?:\d{1,3}\.){3}\d{1,3}|)(?::(?P<port>\d+))?$"
+)
+
 
 class LogLevel(str, Enum):
     info = "info"
@@ -63,10 +68,6 @@ class OpaServerOptions(BaseModel):
         description="list of built-in rego policies and data.json files that must be loaded into OPA on startup. e.g: system.authz policy when using --authorization=basic, see: https://www.openpolicyagent.org/docs/latest/security/#authentication-and-authorization",
     )
 
-    opa_executable_path: str = Field(
-        default="opa", description="Path to the OPA executable"
-    )
-
     class Config:
         use_enum_values = True
         allow_population_by_field_name = True
@@ -81,18 +82,13 @@ def get_cli_options_dict(self):
         """Returns a dict that can be passed to the OPA cli."""
         return self.dict(exclude_none=True, by_alias=True, exclude={"files"})
 
-    def get_opa_startup_files(self) -> str:
-        """Returns a list of startup policies and data."""
-        files = self.files if self.files is not None else []
-        return " ".join(files)
-
 
 class CedarServerOptions(BaseModel):
     """Options to configure the Cedar agent (apply when choosing to run Cedar
     inline)."""
 
     addr: str = Field(
-        ":8181",
+        ":8180",
         description="listening address of the Cedar agent (e.g., [ip]:<port> for TCP)",
     )
     authentication: AuthenticationScheme = Field(
@@ -132,34 +128,23 @@ def validate_authentication_token(cls, v: Optional[str], values: dict[str, Any])
             )
         return v
 
-    def get_cmdline(self) -> str:
-        result = [
-            "cedar-agent",
-        ]
+    def get_args(self) -> Iterable[str]:
         if (
             self.authentication == AuthenticationScheme.token
             and self.authentication_token is not None
         ):
-            result += [
-                "-a",
-                self.authentication_token,
-            ]
-        addr = self.addr.split(":", 1)
-        port = None
-        if len(addr) == 1:
-            listen_address = addr[0]
-        elif len(addr) == 2:
-            listen_address, port = addr
-        if len(listen_address) == 0:
-            listen_address = "0.0.0.0"
-        result += [
-            "--addr",
-            listen_address,
-        ]
-        if port is not None:
-            result += [
-                "--port",
-                port,
-            ]
+            yield "-a"
+            yield self.authentication_token
+
+        match = HOST_ADDR_PATTERN.match(self.addr)
+        if not match:
+            raise ValueError(
+                f"Invalid addr format: {self.addr}. Expected [ip]:<port>, e.g. '0.0.0.0:8180', ':8180'"
+            )
+
+        yield "--addr"
+        yield match.group("addr") or "0.0.0.0"
+        yield "--port"
+        yield match.group("port") or "8180"
+
         # TODO: files
-        return " ".join(result)
diff --git a/packages/opal-client/opal_client/engine/runner.py b/packages/opal-client/opal_client/engine/runner.py
diff --git a/packages/opal-client/opal_client/tests/engine_runner_test.py b/packages/opal-client/opal_client/tests/engine_runner_test.py

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-api-policy-source-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-cedar-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-git-webhook-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-scopes-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`redis:`
`3`	`5`	`image: redis`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-callbacks-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+name: opal-kafka-example`
`1`	`2`
`2`	`3`	`services:`
`3`	`4`	`# Based on: https://developer.confluent.io/quickstart/kafka-docker/`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+name: opal-oauth-example`
	`2`	`+`
`1`	`3`	`services:`
`2`	`4`	`# When scaling the opal-server to multiple nodes and/or multiple workers, we use`
`3`	`5`	`# a broadcast channel to sync between all the instances of opal-server.`