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.

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

Cant use as service in gitlab ci since 3.9 #508

Closed
kucix opened this issue Jul 28, 2021 · 40 comments · Fixed by rabbitmq/rabbitmq-server#3299 or #519
Closed

Cant use as service in gitlab ci since 3.9 #508

kucix opened this issue Jul 28, 2021 · 40 comments · Fixed by rabbitmq/rabbitmq-server#3299 or #519
Labels
question Usability question, not directly related to an error with the image

Comments

@kucix
Copy link

kucix commented Jul 28, 2021

In gitlab-ci.yml we use rabbitmq as service that is configured via ENV props - gitlab does not allow volumes in services.

Example is like this

Acceptance tests:
  image: php
  stage: test
  variables:
    RABBITMQ_ERLANG_COOKIE: xxx""
    RABBITMQ_DEFAULT_USER: "guest"
    RABBITMQ_DEFAULT_PASS: "guest"
    RABBITMQ_DEFAULT_VHOST: "/xxx"
  services:
    - name: rabbitmq:3-management
      alias: rabbitmq
  script:
    - composer tests-acceptance

https://docs.gitlab.com/ee/ci/services/#available-settings-for-services

Now we fixed this by changing to rabbitmq:3.8-management, but what can we do for the future?
How can we configure default user/pass and default vhost?

@Diggsey
Copy link

Diggsey commented Jul 28, 2021

I'm seeing a similar issue.

@wglambert wglambert added the question Usability question, not directly related to an error with the image label Jul 28, 2021
@wglambert
Copy link

That would be due to #467

Reasoning behind the change #506 (comment)

@kucix
Copy link
Author

kucix commented Jul 28, 2021

Yes I know, I have read it, but how can we use version 3.9 and later in gitlab ci as a service and configure it?
That is the question.

@michaelklishin
Copy link
Contributor

This is a great question for the GitLab community. You have several options:

  • Mount or download a rabbitmq.conf with the settings you need and the Erlang cookie file
  • Alternatively the virtual host you set as pre-seeded (default) one can be created at any point using the HTTP API. Same goes for guest's permissions in said virtual host

Setting RABBITMQ_DEFAULT_USER and RABBITMQ_DEFAULT_PASS to guest is unnecessary since those are seed user defaults.

@michaelklishin
Copy link
Contributor

Alternatively you can use definition import, both at boot time and after the node has started (with an HTTP API call), to set up any users, virtual hosts and permissions.

The Definitions doc guide uses rabbitmqadmin, there is nothing wrong
with using POST /api/definitions with a pre-defined definitions (JSON) file body.

@kucix
Copy link
Author

kucix commented Jul 29, 2021

  • Mount or download a rabbitmq.conf with the settings you need and the Erlang cookie file

Yes, if I use rabbitmq as image in which scripts run.
But this is not that case.
In services section of gitlab-ci.yml is mounting not possible.

And if I get it correctly, guest can't connect "remotely" by default, thus can't set vhost via api etc.
Or can?

default:

# guest uses well known
# credentials and can only
# log in from localhost
# by default
loopback_users.guest = true

@joaomcarlos
Copy link

This change has broken hundreds of pipelines for us. Is there no easy setup with environment variables path anymore?

@michaelklishin
Copy link
Contributor

michaelklishin commented Jul 29, 2021

@kucix I suggested using CLI tools or the HTTP API, neither of which has any restrictions for the default user.

You can use RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS to override rabbit.loopback_users:

docker run -it --rm --name rabbitmq -p 5672:5672 -p 15672:15672 -e RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS='-rabbit loopback_users "none"' rabbitmq:3.9-management

if really necessary.

@michaelklishin
Copy link
Contributor

@joaomcarlos most environment variables that were specific to this image (not used by RabbitMQ itself) were intentionally removed.

Your options are outlined above in #508 (comment).

@michaelklishin
Copy link
Contributor

One small feature in RabbitMQ itself that would simplify initial setup for tests would be introducing a load_definitions counterpart that uses a remote URL instead of a local file. That would instantly take care of users/virtual host/permission setup, or maybe even topologies if needed: rabbitmq/rabbitmq-server#3249

Assuming that an HTTP API call against the management plugin-enabled container is possible on GItLab,
I don't see why it would not be possible to import a basic definition file with users/virtual hosts/permissions without any additional features. Then simply use that user
in your suites, creating new users over remote guest access has been the recommended way for many years.

@gerhard
Copy link
Contributor

gerhard commented Jul 29, 2021

Secrets as environment variables is a bad idea, contrary to what 12factor recommends.

RabbitMQ didn't really support these, which is why docker-entrypoint.sh used in this image became the most complex part to the point that most were afraid of changing it. More importantly, this was creating all sorts of weird issues between Docker and Kubernetes. The container startup command should not be generating any just-in-time config from environment variables, because these will clash with what the container scheduler may need to do, especially when it comes to rotating secrets, or declarative topologies (think GitOps).

The defaults work the same, meaning that user guest with pass guest and / vhost CAN login remotely. I just checked with rabbitmq:3-management. This default guest user config makes that possible:

loopback_users.guest = false

If you need to change the default user or vhost, mount a config file.

If you can't mount a file, then declare config properties via RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS , as @michaelklishin describes. It's still possible, just harder, because it's not recommended. By the way, the Erlang cookie can be specified this way as well (again, it's bad idea, but it works).

Also, @michaelklishin proposal in rabbitmq/rabbitmq-server#3249 sounds like a sensible third solution for changing users, vhosts, permissions, etc. What do others think?

@eriksw
Copy link

eriksw commented Jul 30, 2021

Also, @michaelklishin proposal in rabbitmq/rabbitmq-server#3249 sounds like a sensible third solution for changing users, vhosts, permissions, etc. What do others think?

This is not useful unless it supports gs://, s3://, ... etc. (Using ambient creds to access those.)

ECS is a case where there's no way to inject a file, but where it is quite reasonable and well-supported to pass in secrets as environment variables.

Please put the brakes on these anti-environment-variable intentions.

Edit to add:

Please treat the removal of any particular environment variables as being just as breaking to end-users as the removal of a config key from the software itself.

@michaelklishin
Copy link
Contributor

@eriksw why wouldn't downloads over HTTPS (rabbitmq/rabbitmq-server#3249) be sufficient for service container cases? What limits most users to S3 or similar?

@michaelklishin
Copy link
Contributor

@eriksw I strongly disagree that the RabbitMQ core team should treat "removal of any environment variables as breaking as removal of. configuration keys from RabbitMQ itself". This image invented most of the environment variables removed 3.9,
and the fragile, difficult to maintain mechanism that generated a rabbitmq.conf from them, reinventing many things that rabbitmq.conf has supported for years along the way.

All environment variables RabbitMQ supports are still available.

If in some cases, e.g. GitLab or GitHub service containers,
mounting a configuration file is impossible then something new can be introduced. We suggested rabbitmq/rabbitmq-server#3249 as one idea.

@michaelklishin
Copy link
Contributor

@eriksw why wouldn't downloads over HTTPS (rabbitmq/rabbitmq-server#3249) be sufficient for service container cases? What limits most users to S3 or similar?

If I'm reading #506 right, this is because ECS has some kind of limitations (e.g. can only use S3). So two options are:

  • Use a sidecar to seed the node
  • Extend RabbitMQ to fetch definitions from external sources (which can be contributed by those who need them)

While we won't add S3 support to RabbitMQ core in rabbitmq/rabbitmq-server#3249, it can be added
to a small new plugin that would load definitions from external sources, and each individual source can support environment
variables if needed (e.g. AWS access and secret keys). I suspect this plugin would be significantly easier to reason about and
maintain compared to the shell script/environment variable "configuration mechanism" in the 3.8 series of this image.

@michaelklishin
Copy link
Contributor

Specifically for service containers (integration testing), adding three env variables to configure

  • Loopback user list
  • Seed username
  • Seed user password

might be enough for many suites. It won't require any complex shell script in this image and will be an option for non-container users.
The values are either strings or a comma-separated list (or an empty string), which is difficult to screw up.

One argument against these additions is that RabbitMQ uses environment variables only when rabbitmq.conf is not an option (e.g. for the path to the configuration file itself, the node name, and so on).

@gerhard @dumbbell WDYT?

@dumbbell
Copy link

dumbbell commented Aug 9, 2021

Just to make it clear to me, do you suggest RabbitMQ natively supports those three environment variables to override whatever the configuration files say?

@gerhard
Copy link
Contributor

gerhard commented Aug 9, 2021

I think that if we can agree on the 3-4 environment variables that RabbitMQ would need to support natively, then we should implement this directly in RabbitMQ.

Our issue was the complexity of docker-entrypoint.sh, which we have addressed and deprecated since #424. We have been publicly working on this since June 2020. I think that we could have announced it better, and maybe given users more time, before shipping #467. Now that we know what the problem is, we can address it relatively quickly, if we can agree on which environment variables you care about the most @eriksw @kucix @Diggsey @wglambert @joaomcarlos

Would adding support for the following environment variables directly in RabbitMQ solve this issue?

  • RABBITMQ_DEFAULT_USER
  • RABBITMQ_DEFAULT_PASS
  • RABBITMQ_DEFAULT_VHOST
  • RABBITMQ_ERLANG_COOKIE

@kucix
Copy link
Author

kucix commented Aug 9, 2021

For our purposes yes. These four variables.

@eriksw
Copy link

eriksw commented Aug 9, 2021

This image invented most of the environment variables removed 3.9...

As an end user, I do not care whether the environment variables are invented or implemented by RabbitMQ itself or by an entrypoint script.

I care about being able to configure these things (and everything) using environment variables, without the burden of having to maintain building and distributing a custom image.

I believe it is an important, nearly-fundamental purpose of an official library container image to do what it reasonably can to paper over and correct for things that have awful UX in the container ecosystem, such as file-based logging and config files.

I think that we could have announced it better, and maybe given users more time, before shipping #467.

In the future I'd suggest:

  • Crash if deprecated configuration is set but an acknowledgment to re-enable it is not set.
  • In the message, direct people to file issues if they have use cases that require them to re-enable the deprecated functionality.

Had a process like that been followed, I think it's pretty reasonable to assume there would have been ample feedback here about the need to configure these things using environment variables; that config files are impractical or impossible to inject in many real-world circumstances.

@eriksw why wouldn't downloads over HTTPS (rabbitmq/rabbitmq-server#3249) be sufficient for service container cases? What limits most users to S3 or similar?

When running in a managed environment like ECS, the obvious place to store a file is the cloud provider's object storage service. Unless world-readable, that requires authorization to access—authorization you would grant to the ambient credential identity available to the running container, which means acting as a fully-fledged client of the various storage services and their various credential-discovery mechanisms.

Edit to add: This is relevant when the file contains sensitive values. If the file is one that doesn't need to contain any sensitive values, such that it'd be totally fine to serve the file in world-readable way, then yes, it would be unusual for there to be anything interfering with outbound access to load from an ordinary https url. (Notwithstanding the poor UX of working at the granularity of the entire file, instead of setting particular items.)

I should have been more clear: I don't think it's appropriate for RabbitMQ to support loading config files from all the various cloud storage providers. (It would be an arguably absurd, ever-growing amount of complexity and dependencies, certainly far worse than even a fully generic configure-anything-via-envar system.) I brought it up to try and illustrate how lacking the config/definitions-from-URL concept is when applied to the practicalities of environments where file mounts are a non-starter.

I appreciate the desire to keep the entrypoint maintainable, but if the RabbitMQ team itself isn't interested in being thoroughly configurable using environment variables, then the maintainers of this image are still in the next-best position to maintain a workaround to mitigate upstream unwillingness/disinterest.

@gerhard
Copy link
Contributor

gerhard commented Aug 10, 2021

I am trying to empathise with your perspective @eriksw, but the negative undertones make it difficult. I understand where you are coming from, thank you for your input, and encourage you to think about the wider RabbitMQ ecosystem. Amazon MQ for RabbitMQ is an interesting perspective, as is the cluster-operator & messaging-topology-operator. We will do our best to make this work within the wider context, across multiple container images (this is just one of them), and the sprawling container orchestration landscape.

Thanks @kucix for confirming the environment variables which are most important to you.

What do you think @michaelklishin & @dumbbell, is it a good idea to handle these within RabbitMQ itself?

@dumbbell
Copy link

Yes, I think we can do that. I don't know much about the usage in containers, but I find environment variables really handy while testing and debugging.

I will prepare a patch and keep you posted.

dumbbell added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
They are the equivalent of the `default_{user,pass,vhost}` configuration
settings. Each set environment variable will take precedence over its
configuration file counterpart.

Fixes docker-library/rabbitmq#508.
dumbbell added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
It is the equivalent of the content of the Erlang cookie file. Note this
variable IS the cookie value, NOT the path to a cookie file.

If it is set, it will take precedence over the content of the Erlang
cookie file.

Fixes docker-library/rabbitmq#508.
@dumbbell
Copy link

Hi!

The patch is ready for wider testing (see rabbitmq/rabbitmq-server#3299).

CI should produce a Docker image. Could you please give it a try when that image appears? I don't know how to explain how to use the image built from this branch (I don't know it myself) and, as I use Docker once in a blue Moon, you will probably figure out way before me how to fetch and use that image :-)

dumbbell added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
They are the equivalent of the `default_{user,pass,vhost}` configuration
settings. Each set environment variable will take precedence over its
configuration file counterpart.

Fixes docker-library/rabbitmq#508.
dumbbell added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
It is the equivalent of the content of the Erlang cookie file. Note this
variable IS the cookie value, NOT the path to a cookie file.

If it is set, it will take precedence over the content of the Erlang
cookie file.

Fixes docker-library/rabbitmq#508.
@michaelklishin
Copy link
Contributor

michaelklishin commented Aug 11, 2021

We should mention that these default credentials and virtual host are seeded only on first node boot but the cookie will be read and used on every node start. This can potentially be important in some environments.

@michaelklishin
Copy link
Contributor

To pull the automatically built image for the above PR prepared by @dumbbell, use

docker pull pivotalrabbitmq/rabbitmq:add-env-vars-to-set-default-user-pass-vhost-and-erlang-cookie-otp-max

I don't know if there may be any public access limitations, though. The Docker Hub account was originally set up for the needs of the RabbitMQ core team.

@michaelklishin
Copy link
Contributor

I tried

docker run -it --rm --name rabbitmq-pr-3299 -p 5672:5672 -p 15672:15672 -e RABBITMQ_DEFAULT_USER="pr-3299" -e RABBITMQ_DEFAULT_PASS="pr-3299" pivotalrabbitmq/rabbitmq:add-env-vars-to-set-default-user-pass-vhost-and-erlang-cookie-otp-max

and the image refuses to start:

error: RABBITMQ_DEFAULT_PASS is set but deprecated
error: RABBITMQ_DEFAULT_USER is set but deprecated
error: deprecated environment variables detected

so after the above PR is merged and a new release ships, we'd have to whitelist those four variables.

@kucix
Copy link
Author

kucix commented Aug 11, 2021

Sorry, that image is not working. It ends on deprecated error and stops.

image

@michaelklishin
Copy link
Contributor

This is because the entrypoint is not aware of the fact that RABBITMQ_DEFAULT_USER and such are/can be/will be supported by RabbitMQ itself.

@michaelklishin
Copy link
Contributor

michaelklishin commented Aug 11, 2021

Filed #513 which can be addressed when RabbitMQ 3.9.4 ships (should be in a week or two).

@tianon
Copy link
Member

tianon commented Aug 11, 2021

You'll lose a tiny amount of functionality (the few bits the entrypoint still does), but you should be able to test with that image via --entrypoint rabbitmq-server (probably want to also include --user rabbitmq or somesuch).

@kucix
Copy link
Author

kucix commented Aug 11, 2021

No, I cant.
I!m testing scenario from first post - gitlab ci service.

mergify bot pushed a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
They are the equivalent of the `default_{user,pass,vhost}` configuration
settings. Each set environment variable will take precedence over its
configuration file counterpart.

Fixes docker-library/rabbitmq#508.

(cherry picked from commit 46b8321)
mergify bot pushed a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 11, 2021
It is the equivalent of the content of the Erlang cookie file. Note this
variable IS the cookie value, NOT the path to a cookie file.

If it is set, it will take precedence over the content of the Erlang
cookie file.

Fixes docker-library/rabbitmq#508.

(cherry picked from commit bd39027)
@michaelklishin
Copy link
Contributor

Those testing with local Docker can use --entrypoint and --user, thank you @tianon.

@gerhard
Copy link
Contributor

gerhard commented Aug 12, 2021

This is a great first step, and we have definitely addressed a big part of the problem, but we still have a few issues left in the current pivotalrabbitmq/rabbitmq:v3.9.x-otp-max, specifically pivotalrabbitmq/rabbitmq:b94ca39a24bd84ca7e2f1fd252bf3b4e12bc5c06-otp-max

1/3. Default user cannot log in remotely

If the default user gets changed, then the loopback_users.guest = false should change too.

Would you agree @dumbbell ?

2/3. Defining an Erlang cookie triggers a warning

RABBITMQ_ERLANG_COOKIE env variable support is deprecated and will be REMOVED in a future version. Use the $HOME/.erlang.cookie file or the --erlang-cookie switch instead.

This was introduced via rabbitmq/rabbitmq-server@d7ca0e9#diff-8fcb6fa84456486ea48b2c2c13a2505f053cfa5ab13604208a400b86719fe243, and if we now support it, we should go back and revisit it.

3/3. Default entrypoint does not work

This is a pretty big one. If we genuinely want to support these environment variables, then we should remove the now supported environment variables from

deprecatedEnvVars=(
RABBITMQ_DEFAULT_PASS
RABBITMQ_DEFAULT_PASS_FILE
RABBITMQ_DEFAULT_USER
RABBITMQ_DEFAULT_USER_FILE
RABBITMQ_DEFAULT_VHOST
RABBITMQ_ERLANG_COOKIE
RABBITMQ_MANAGEMENT_SSL_CACERTFILE
RABBITMQ_MANAGEMENT_SSL_CERTFILE
RABBITMQ_MANAGEMENT_SSL_DEPTH
RABBITMQ_MANAGEMENT_SSL_FAIL_IF_NO_PEER_CERT
RABBITMQ_MANAGEMENT_SSL_KEYFILE
RABBITMQ_MANAGEMENT_SSL_VERIFY
RABBITMQ_SSL_CACERTFILE
RABBITMQ_SSL_CERTFILE
RABBITMQ_SSL_DEPTH
RABBITMQ_SSL_FAIL_IF_NO_PEER_CERT
RABBITMQ_SSL_KEYFILE
RABBITMQ_SSL_VERIFY
RABBITMQ_VM_MEMORY_HIGH_WATERMARK
)

Would you agree with this @tianon?

I was not around when #467 was merged, but I would agree that it's less a deprecation, and more a removal. Updating the variables to reflect this seems sensible to me. I can see now that us releasing 3.9.0 accelerated those changes, and in hindsight combining them doesn't seem right. We are now having to deal with the side-effects of this which is not great, but I am confident that we will be able to restore the essential functionality shortly, without needing to revert #467.


As an aside, pivotalrabbitmq/rabbitmq are dev container images which track this image closely, the only difference is that they enable all plugins and are built for every single commit via https://github.com/rabbitmq/rabbitmq-server/blob/master/.github/workflows/oci.yaml

@dumbbell
Copy link

1/3. Default user cannot log in remotely

If the default user gets changed, then the loopback_users.guest = false should change too.

The new environment variable just overrides the value of the default_user configuration parameter. The behavior behind this parameter didn't change with the patch. So currently, if one sets default_user in the configuration or use $RABBITMQ_DEFAULT_USER, it doesn't interfere with loopback_users.

The value of loopback_users is set independently of default_user. In other words, its default value is set to <<"guest">>, regardless of the value of default_user.

That said, if you think this behavior is wrong, we can change it. I agree it would make sense that loopback_users defaults to whatever default_user is.

@gerhard
Copy link
Contributor

gerhard commented Aug 12, 2021

The value of loopback_users is set independently of default_user. In other words, its default value is set to <<"guest">>, regardless of the value of default_user.

That said, if you think this behavior is wrong, we can change it. I agree it would make sense that loopback_users defaults to whatever default_user is.

We may need to introduce another environment variable, so that we wouldn't need to do this: https://github.com/docker-library/rabbitmq/blob/master/10-default-guest-user.conf.

From an end-user perspective, the problem that we are trying to solve is to allow the default user & password to be configurable via environment variables. That user should be able to login via any network interface by default. The implementation is up to the person that writes the code 🙂

@michaelklishin
Copy link
Contributor

michaelklishin commented Aug 13, 2021

Loopback user limitation exists to make sure that a node with the default seeded user (with widely known credentials) does not accept remote connections. If you override seed user credentials, sounds like the most basic "scan and try default credentials" kind of attack won't be effective by definition.

So let's keep the scope of the changes as is and not touch loopback user list defaults?

@gerhard
Copy link
Contributor

gerhard commented Aug 13, 2021

I'm all for leaving the scope as is, and not expanding it with loopback_users. RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS can be used for that purpose if needed.

2/3 and 3/3 still seem to be in scope to me. Would you agree @michaelklishin?

@yosifkit
Copy link
Member

Yes, the entrypoint is easy to change and if RabbitMQ itself begins to support a variable of those names, they can be removed from the list.

@michaelklishin
Copy link
Contributor

@gerhard yes, definitely, we will have to adjust the image. I have earlier filed #513 for that.

Note that RabbitMQ intentionally logs when the cookie value is overridden using an env variable: I assumed that this must be a warning as this new variable unfortunately can be used as an attack vector, so it should be very obvious from the logs what the cookie value source is.

@gerhard
Copy link
Contributor

gerhard commented Aug 14, 2021

OK, sounds good, I'll be back in a week 👋🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Usability question, not directly related to an error with the image
Projects
None yet
10 participants