Tighten storage-operator permissions against Salt

eve/create-volumes.sh

@@ -59,28 +59,33 @@ sed "s/NODE_NAME/${NODE_NAME}/" \
     "${PRODUCT_MOUNT}/examples/prometheus-sparse.yaml" | \
     kubectl apply -f -
 
-echo "Waiting for PV '$NODE_NAME-alertmanager' to be provisioned"
-if ! retry "$MAX_TRIES" check_pv_exists "$NODE_NAME-alertmanager"; then
-    echo "PV not created"
-    exit 1
-fi
+wait_for_pv() {
+    local -r pv="$1"
+
+    echo "Waiting for PV '$pv' to be provisioned"
+    if ! retry "$MAX_TRIES" check_pv_exists "$pv"; then
+        echo "PV '$pv' not created"
+        kubectl logs -n kube-system deploy/storage-operator
+        exit 1
+    fi
+}
 
-echo "Waiting for PV '$NODE_NAME-prometheus' to be provisioned"
-if ! retry "$MAX_TRIES" check_pv_exists "$NODE_NAME-prometheus"; then
-    echo "PV not created"
-    exit 1
-fi
+wait_for_pv "$NODE_NAME-alertmanager"
+wait_for_pv "$NODE_NAME-prometheus"
 
-echo 'Waiting for AlertManager to be running'
-if ! retry "$MAX_TRIES" check_pod_is_in_phase metalk8s-monitoring \
-      alertmanager-prometheus-operator-alertmanager-0 Running; then
-    echo "AlertManager is not Running"
-    exit 1
-fi
+wait_for_pod() {
+    local -r name="$1" namespace="$2" pod="$3"
 
-echo 'Waiting for Prometheus to be running'
-if ! retry "$MAX_TRIES" check_pod_is_in_phase metalk8s-monitoring \
-      prometheus-prometheus-operator-prometheus-0 Running; then
-    echo "Prometheus is not Running"
-    exit 1
-fi
+    echo "Waiting for $name to be running"
+    if ! retry "$MAX_TRIES" check_pod_is_in_phase "$namespace" "$pod" Running
+    then
+        echo "$name is not Running"
+        kubectl describe pod -n "$namespace" "$pod"
+        exit 1
+    fi
+}
+
+wait_for_pod "AlertManager" \
+    metalk8s-monitoring alertmanager-prometheus-operator-alertmanager-0
+wait_for_pod "Prometheus" \
+    metalk8s-monitoring prometheus-prometheus-operator-prometheus-0

salt/metalk8s/salt/master/files/master-99-metalk8s.conf.j2

@@ -45,7 +45,9 @@ external_auth:
     storage-operator:
       - '*':
         - 'disk.dump'
-        - 'state.sls'
+        - 'state.sls':
+            kwargs:
+              mods: 'metalk8s\.volumes.*'
       - '@jobs'
 
 # `kubeconfig` file and `context` used by salt to interact with apiserver

storage-operator/deploy/role.yaml

@@ -4,36 +4,63 @@ metadata:
   creationTimestamp: null
   name: storage-operator
 rules:
+# For recording transition events
 - apiGroups:
   - ""
   resources:
-  - pods
-  - nodes
-  - services
-  - endpoints
-  - persistentvolumes
-  - persistentvolumeclaims
   - events
-  - configmaps
-  - secrets
   verbs:
-  - '*'
+  - create
+  - patch
+# For setting up monitoring for itself
+- apiGroups:
+  - ""
+  resources:
+  - services
+  verbs:
+  - get
+  - create
+  - update
 - apiGroups:
   - apps
   resources:
   - deployments
-  - daemonsets
   - replicasets
-  - statefulsets
   verbs:
-  - '*'
+  - get
 - apiGroups:
   - monitoring.coreos.com
   resources:
   - servicemonitors
   verbs:
   - get
   - create
+# For reading its own name and namespace
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - get
+# For managing its lock
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  resourceNames:
+  - storage-operator-lock
+  verbs:
+  - get
+  - update
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  # NOTE: cannot scope "create" to a resourceName, see
+  # https://kubernetes.io/docs/reference/access-authn-authz/rbac/#referring-to-resources
+  - create
+# For managing its own graceful termination
 - apiGroups:
   - apps
   resourceNames:
@@ -42,24 +69,32 @@ rules:
   - deployments/finalizers
   verbs:
   - update
+# For managing owned PVs
 - apiGroups:
   - ""
   resources:
-  - pods
+  - persistentvolumes
   verbs:
-  - get
+  - '*'
+# For reading a Node's MetalK8s version
 - apiGroups:
-  - apps
+  - ""
   resources:
-  - replicasets
+  - nodes
   verbs:
   - get
+  # NOTE: we only use "get" in code, but the controller-runtime tooling uses
+  #       "list" and "watch" to manage a cache
+  - list
+  - watch
+# For every custom resource from this Operator
 - apiGroups:
   - storage.metalk8s.scality.com
   resources:
   - '*'
   verbs:
   - '*'
+# For reading device preparation details (formatting and mounting options)
 - apiGroups:
   - storage.k8s.io
   resources:

tests/post/features/salt_api.feature

@@ -12,8 +12,10 @@ Feature: SaltAPI
     Scenario: Login to SaltAPI using a ServiceAccount
         Given the Kubernetes API is available
         When we login to SaltAPI with the ServiceAccount 'storage-operator'
-        Then we can invoke '["disk.dump", "state.sls"]' on '*'
+        Then we can invoke '["disk.dump", {"state.sls": {"kwargs": {"mods": r"metalk8s\.volumes.*"}}}]' on '*'
         And we have '@jobs' perms
+        And we can not ping all minions
+        And we can not run state 'test.nop' on '*'
 
     Scenario: Login to SaltAPI using an incorrect password
         Given the Kubernetes API is available

tests/post/steps/test_salt_api.py

@@ -8,6 +8,17 @@
 from pytest_bdd import parsers, scenario, then, when
 
 
+def _negation(value):
+    """Parse an optional negation after a verb (in a Gherkin feature spec)."""
+    if value == "":
+        return False
+    elif value in [" not", "not"]:
+        return True
+    else:
+        raise ValueError(
+            "Cannot parse '{}' as an optional negation".format(value)
+        )
+
 # Scenario {{{
 
 
@@ -71,26 +82,34 @@ def login_salt_api_token(host, k8s_client, account_name, version, context):
 # Then {{{
 
 
-@then('we can ping all minions')
-def ping_all_minions(host, context):
-    result = requests.post(
-        context['salt-api']['url'],
-        json=[
-            {
-                'client': 'local',
-                'tgt': '*',
-                'fun': 'test.ping',
-            },
-        ],
-        headers={
-            'X-Auth-Token': context['salt-api']['token'],
-        },
-        verify=False,
-    )
+@then(parsers.cfparse(
+    'we can{negated:Negation?} ping all minions',
+    extra_types={'Negation': _negation}
+))
+def ping_all_minions(host, context, negated):
+    result = _salt_call(context, 'test.ping', tgt='*')
+
+    if negated:
+        assert result.status_code == 401
+        assert 'No permission' in result.text
+    else:
+        result_data = result.json()
+        assert result_data['return'][0] != []
 
-    result_data = result.json()
 
-    assert result_data['return'][0] != []
+@then(parsers.cfparse(
+    "we can{negated:Negation?} run state '{module}' on '{targets}'",
+    extra_types={'Negation': _negation}
+))
+def run_state_on_targets(host, context, negated, module, targets):
+    result = _salt_call(context, 'state.sls', tgt=targets,
+                        kwarg={'mods': module})
+
+    if negated:
+        assert result.status_code == 401
+        assert 'No permission' in result.text
+    else:
+        assert result.status_code == 200
 
 
 @then('authentication fails')
@@ -164,4 +183,25 @@ def _salt_api_login(address, username=None, password=None, token=None):
     return result
 
 
+def _salt_call(context, fun, tgt='*', arg=None, kwarg=None):
+    action = {
+        'client': 'local',
+        'tgt': tgt,
+        'fun': fun,
+    }
+    if arg is not None:
+        action['arg'] = arg
+    if kwarg is not None:
+        action['kwarg'] = kwarg
+
+    return requests.post(
+        context['salt-api']['url'],
+        json=[action],
+        headers={
+            'X-Auth-Token': context['salt-api']['token'],
+        },
+        verify=False,
+    )
+
+
 # }}}