Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

rabbitmq_binding breaks idempotency if arguments is not empty #160

Open
oneoneonepig opened this issue Oct 30, 2023 · 0 comments · May be fixed by #161
Open

rabbitmq_binding breaks idempotency if arguments is not empty #160

oneoneonepig opened this issue Oct 30, 2023 · 0 comments · May be fixed by #161

Comments

@oneoneonepig
Copy link

SUMMARY

When a binding is created, it will calculate the properties_key based on the routing_key and arguments:

# pseudo code
if len(arguments) == 0 and routing_key == "":
  properties_key = "~"
else if len(arguments) == 0 and routing_key != "":
  properties_key = routing_key
else if len(arguments) > 0:
  properties_key = f"{routing_key}~{hash(arguments)}"

The actual hash() algorithm will be mentioned later

When rabbitmq_binding checks if the resource exists, it invokes the following request:

# https://github.com/ansible-collections/community.rabbitmq/blob/146b006de99112e1abb5c2be0c4f2bcc8d6c3a5f/plugins/modules/rabbitmq_binding.py#L114-L123

        self.props = urllib_parse.quote(self.routing_key) if self.routing_key != '' else '~'
        self.base_url = '{0}://{1}:{2}/api/bindings'.format(self.login_protocol,
                                                            self.login_host,
                                                            self.login_port)
        self.url = '{0}/{1}/e/{2}/{3}/{4}/{5}'.format(self.base_url,
                                                      urllib_parse.quote(self.vhost, safe=''),
                                                      urllib_parse.quote(self.name, safe=''),
                                                      self.destination_type,
                                                      urllib_parse.quote(self.destination, safe=''),
                                                      self.props)

The logic does not consider arguments. As a result, bindings with arguments will not have idempotency and will introduce some incorrect behavior.

The check uses the HTTP API /api/bindings/vhost/e/exchange/q/queue/props, which describes as below:

An individual binding between an exchange and a queue. The props part of the URI is a "name" for the binding composed of its routing key and a hash of its arguments. props is the field named "properties_key" from a bindings listing response.

The following snippet describes the actual hash algorithm:

%% https://github.com/rabbitmq/rabbitmq-server/blob/70047f52453c37d8f3fa185807c8f574dd95ef71/deps/rabbitmq_management_agent/src/rabbit_mgmt_format.erl#L412-L424
binding(#binding{source      = S,
                 key         = Key,
                 destination = D,
                 args        = Args}) ->
    format(
      [{source,           S},
       {destination,      D#resource.name},
       {destination_type, D#resource.kind},
       {routing_key,      Key},
       {arguments,        Args},
       {properties_key, pack_binding_props(Key, Args)}],
      {fun format_binding/1, false}).

%% https://github.com/rabbitmq/rabbitmq-server/blob/70047f52453c37d8f3fa185807c8f574dd95ef71/deps/rabbitmq_management_agent/src/rabbit_mgmt_format.erl#L336-L342
pack_binding_props(<<"">>, []) ->
    <<"~">>;
pack_binding_props(Key, []) ->
    list_to_binary(quote_binding(Key));
pack_binding_props(Key, Args) ->
    ArgsEnc = args_hash(Args),
    list_to_binary(quote_binding(Key) ++ "~" ++ quote_binding(ArgsEnc)).

%% https://github.com/rabbitmq/rabbitmq-server/blob/70047f52453c37d8f3fa185807c8f574dd95ef71/deps/rabbitmq_management_agent/src/rabbit_mgmt_format.erl#L605-L606
args_hash(Args) ->
    list_to_binary(rabbit_misc:base64url(<<(erlang:phash2(Args, 1 bsl 32)):32>>)).

%% https://github.com/rabbitmq/rabbitmq-server/blob/70047f52453c37d8f3fa185807c8f574dd95ef71/deps/rabbit_common/src/rabbit_misc.erl#L1120-L1125
base64url(In) ->
    lists:reverse(lists:foldl(fun ($\+, Acc) -> [$\- | Acc];
                                  ($\/, Acc) -> [$\_ | Acc];
                                  ($\=, Acc) -> Acc;
                                  (Chr, Acc) -> [Chr | Acc]
                              end, [], base64:encode_to_string(In))).

I do not write Erlang, let me know if I am grabbing the wrong blocks

Suggested fixes:

  1. Instead of using self.http_check_states.get(self.api_result.status_code, False), retrieve all bindings via /api/bindings/vhost/e/exchange/q/queue and check if the resource exists,
  2. Use the same method to create the properties_key and place it in URL when invoking /api/bindings/vhost/e/exchange/q/queue/props
ISSUE TYPE
  • Bug Report
COMPONENT NAME

community.rabbitmq.rabbitmq_binding

ANSIBLE VERSION
ansible [core 2.15.3]
  config file = /var/user/ansible/ansible.cfg
  configured module search path = ['/home/jeffrey/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jeffrey/.local/lib/python3.10/site-packages/ansible
  ansible collection location = /home/jeffrey/.ansible/collections
  executable location = /home/jeffrey/.local/bin/ansible
  python version = 3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0] (/usr/bin/python3)
  jinja version = 3.0.3
  libyaml = True
COLLECTION VERSION
# /home/jeffrey/.ansible/collections/ansible_collections
Collection         Version
------------------ -------
community.rabbitmq 1.2.3
CONFIGURATION

Omitted

OS / ENVIRONMENT

OS: Ubuntu 22.04.3 LTS
Kernel: 5.15.0-76-generic
RabbitMQ: 3.9.13-1ubuntu0.22.04.1 (current latest release with Ubuntu 22.04)

STEPS TO REPRODUCE

Run the following tasks at least once:

    - name: Bind queue to exchange rk/ arg/
      community.rabbitmq.rabbitmq_binding:
        name: xexchange
        destination: xqueue
        type: queue
        routing_key: ""
    - name: Bind queue to exchange rk/aaa arg/AAA
      community.rabbitmq.rabbitmq_binding:
        name: xexchange
        destination: xqueue
        type: queue
        routing_key: "aaa"
        arguments:
          type: "AAA"
    - name: Bind queue to exchange rk/bbb arg/
      community.rabbitmq.rabbitmq_binding:
        name: xexchange
        destination: xqueue
        type: queue
        routing_key: "bbb"
    - name: Bind queue to exchange rk/# arg/CCC
      community.rabbitmq.rabbitmq_binding:
        name: xexchange
        destination: xqueue
        type: queue
        arguments:
          type: "CCC"
    - name: Bind queue to exchange rk/ arg/DDD
      community.rabbitmq.rabbitmq_binding:
        name: xexchange
        destination: xqueue
        type: queue
        routing_key: ""
        arguments:
          type: "DDD"
EXPECTED RESULTS

After the first run, all consecutive runs should report ok: [...] instead of changed: [...]

ACTUAL RESULTS
TASK [Bind queue to exchange rk/ arg/] *************************************************
ok: [lab1]

TASK [Bind queue to exchange rk/aaa arg/AAA] *******************************************
changed: [lab1]

TASK [Bind queue to exchange rk/bbb arg/] **********************************************
ok: [lab1]

TASK [Bind queue to exchange rk/# arg/CCC] *********************************************
changed: [lab1]

TASK [Bind queue to exchange rk/ arg/DDD] **********************************************
ok: [lab1]
case routing_key arguments idempotency properties_key
1 ok finds "~", and it is "~"
2 aaa type:AAA changed finds "aaa", but actually "aaa~xxxx"
3 bbb ok finds "bbb", and it is "bbb"
4 # type:CCC changed finds "#", and actually "#~xxxx", similar to case 2
5 type:DDD ok (wrong!) finds "~", found "~", but actually matches the binding created by case 1

Invoke the HTTP API and see the properties_key:
curl -u guest -s http://localhost:15672/api/bindings/%2F/e/xexchange/q/xqueue | jq

[
  {
    "source": "xexchange",
    "vhost": "/",
    "destination": "xqueue",
    "destination_type": "queue",
    "routing_key": "",
    "arguments": {},
    "properties_key": "~"
  },
  {
    "source": "xexchange",
    "vhost": "/",
    "destination": "xqueue",
    "destination_type": "queue",
    "routing_key": "",
    "arguments": {
      "type": "DDD"
    },
    "properties_key": "~7V4gwQ"
  },
  {
    "source": "xexchange",
    "vhost": "/",
    "destination": "xqueue",
    "destination_type": "queue",
    "routing_key": "#",
    "arguments": {
      "type": "CCC"
    },
    "properties_key": "%23~fF974A"
  },
  {
    "source": "xexchange",
    "vhost": "/",
    "destination": "xqueue",
    "destination_type": "queue",
    "routing_key": "aaa",
    "arguments": {
      "type": "AAA"
    },
    "properties_key": "aaa~yG2vtw"
  },
  {
    "source": "xexchange",
    "vhost": "/",
    "destination": "xqueue",
    "destination_type": "queue",
    "routing_key": "bbb",
    "arguments": {},
    "properties_key": "bbb"
  }
]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes rabbitmq_binding breaks idempotency if arguments is not empty oneoneonepig/community.rabbitmq
1 participant
@oneoneonepig