Merge pull request #8 from nicholasmhughes/port-pr-62598

fix Rules attribute formatting to allow for proper creation

changelog/7.fixed.md

@@ -0,0 +1 @@
+fixed Rules attribute formatting to allow for proper creation

docs/ref/modules/index.rst

@@ -10,4 +10,3 @@ _________________
     :toctree:
 
     consul
-    consul_mod

docs/ref/states/index.rst

@@ -10,4 +10,3 @@ _____________
     :toctree:
 
     consul
-    consul_mod

src/saltext/consul/modules/consul.py

@@ -44,6 +44,8 @@ def _query(
     api_version="v1",
     data=None,
     query_params=None,
+    decode=True,
+    text=False,
 ):
     """
     Consul object method function to construct and execute on the API URL.
@@ -53,6 +55,8 @@ def _query(
     :param function:    The Consul api function to perform.
     :param method:      The HTTP method, e.g. GET or POST.
     :param data:        The data to be sent for POST method. This param is ignored for GET requests.
+    :param decode:      Passed to salt.utils.http.query to decode the output into a data structure.
+    :param text:        Passed to salt.utils.http.query to return the raw response body in the "text" key.
     :return:            The json response from the API call or False.
     """
 
@@ -82,7 +86,8 @@ def _query(
         method=method,
         params=query_params,
         data=data,
-        decode=True,
+        decode=decode,
+        text=text,
         status=True,
         header_dict=headers,
         opts=__opts__,
@@ -977,7 +982,7 @@ def agent_check_fail(consul_url=None, token=None, checkid=None, **kwargs):
     return ret
 
 
-def agent_service_register(consul_url=None, token=None, **kwargs):
+def agent_service_register(consul_url=None, token=None, decode=False, text=True, **kwargs):
     """
     The used to add a new service, with an optional
     health check, to the local agent.
@@ -1000,6 +1005,8 @@ def agent_service_register(consul_url=None, token=None, **kwargs):
                       endpoint must be used periodically to update
                       the state of the check.
     :param check_interval: Interval at which the check should run.
+    :param decode: Passed to salt.utils.http.query to decode the output into a data structure.
+    :param text: Passed to salt.utils.http.query to return the raw response body in the "text" key.
     :return: Boolean and message indicating success or failure.
 
     CLI Example:
@@ -1077,7 +1084,15 @@ def agent_service_register(consul_url=None, token=None, **kwargs):
             data["Check"] = check_dd  # if empty, ignore it
 
     function = "agent/service/register"
-    res = _query(consul_url=consul_url, function=function, token=token, method="PUT", data=data)
+    res = _query(
+        consul_url=consul_url,
+        function=function,
+        token=token,
+        method="PUT",
+        data=data,
+        decode=decode,
+        text=text,
+    )
     if res["res"]:
         ret["res"] = True
         ret[
@@ -1095,12 +1110,14 @@ def agent_service_register(consul_url=None, token=None, **kwargs):
     return ret
 
 
-def agent_service_deregister(consul_url=None, token=None, serviceid=None):
+def agent_service_deregister(consul_url=None, token=None, serviceid=None, decode=False, text=True):
     """
     Used to remove a service.
 
     :param consul_url: The Consul server URL.
     :param serviceid: A serviceid describing the service.
+    :param decode: Passed to salt.utils.http.query to decode the output into a data structure.
+    :param text: Passed to salt.utils.http.query to return the raw response body in the "text" key.
     :return: Boolean and message indicating success or failure.
 
     CLI Example:
@@ -2080,9 +2097,6 @@ def acl_create(consul_url=None, token=None, **kwargs):
             ret["res"] = False
             return ret
 
-    if "id" in kwargs:
-        data["id"] = kwargs["id"]
-
     if "name" in kwargs:
         data["Name"] = kwargs["name"]
     else:
@@ -2092,7 +2106,15 @@ def acl_create(consul_url=None, token=None, **kwargs):
         data["Type"] = kwargs["type"]
 
     if "rules" in kwargs:
-        data["Rules"] = kwargs["rules"]
+        rules_str = ""
+        rules = kwargs["rules"]
+        for item in rules:
+            for key, val in item.items():
+                if key != "policy":
+                    rules_str += f'{key} "{val}" {{\n'
+                else:
+                    rules_str += f'  {key} = "{val}"\n}}\n'
+        data["Rules"] = rules_str
 
     function = "acl/create"
     res = _query(consul_url=consul_url, token=token, data=data, method="PUT", function=function)
@@ -2225,6 +2247,7 @@ def acl_delete(consul_url=None, token=None, **kwargs):
         ] = "Removing ACL {} failed.".format(  # pylint: disable=consider-using-f-string
             kwargs["id"]
         )
+        ret["changes"] = res
 
     return ret
 

src/saltext/consul/states/consul.py

@@ -10,11 +10,19 @@
 
     acl_present:
       consul.acl_present:
-        - id: 38AC8470-4A83-4140-8DFD-F924CD32917F
         - name: acl_name
-        - rules: node "" {policy = "write"} service "" {policy = "read"} key "_rexec" {policy = "write"}
-        - type: client
         - consul_url: http://localhost:8500
+        - rules:
+          - node: "host.example.local"
+            policy: "write"
+          - agent: "host.example.local"
+            policy: "write"
+          - session: "host.example.local"
+            policy: "write"
+          - key: ""
+            policy: "read"
+          - service: ""
+            policy: "read"
 
     acl_delete:
        consul.acl_absent:

tests/unit/modules/test_consul.py

@@ -1471,12 +1471,52 @@ def test_acl_create():
                 consul_url=consul_url,
             )
 
-    with patch.object(salt.utils.http, "query", return_value=mock_http_result):
+    mock_http_query = MagicMock(return_value=mock_http_result)
+    with patch.object(salt.utils.http, "query", mock_http_query):
         with patch.dict(consul.__salt__, {"config.get": mock_url}):
             result = consul.acl_create(consul_url=consul_url, token=token, name=name)
             expected = {"message": f"ACL {name} created.", "res": True}
             assert expected == result
 
+            mock_http_query.reset_mock()
+
+            # acl_create with rules
+            rules = [
+                {
+                    "node": "host.example.local",
+                    "policy": "write",
+                },
+                {
+                    "agent": "host.example.local",
+                    "policy": "write",
+                },
+                {
+                    "session": "host.example.local",
+                    "policy": "write",
+                },
+                {
+                    "key": "",
+                    "policy": "read",
+                },
+                {
+                    "service": "",
+                    "policy": "read",
+                },
+            ]
+            result = consul.acl_create(consul_url=consul_url, token=token, name=name, rules=rules)
+            mock_http_query.assert_called_with(
+                "http://localhost:1313/v1/acl/create",
+                method="PUT",
+                params={},
+                data='{"Name": "name1", "Rules": [{"node": "host.example.local", "policy": "write"}, {"agent": "host.example.local", "policy": "write"}, {"session": "host.example.local", "policy": "write"}, {"key": "", "policy": "read"}, {"service": "", "policy": "read"}]}',
+                decode=True,
+                status=True,
+                header_dict={"X-Consul-Token": "randomtoken", "Content-Type": "application/json"},
+                opts={"consul": {"url": "http://127.0.0.1", "token": "test_token"}},
+            )
+            expected = {"message": f"ACL {name} created.", "res": True}
+            assert expected == result
+
 
 def test_acl_update():
     """