Skip to content

Remove support for setting content of gitlab-secrets.json #213

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

Closed
LongLiveCHIEF opened this issue Apr 9, 2018 · 5 comments · Fixed by #249
Closed

Remove support for setting content of gitlab-secrets.json #213

LongLiveCHIEF opened this issue Apr 9, 2018 · 5 comments · Fixed by #249
Labels
backwards-incompatible needs-feedback Further information is requested
Milestone

Comments

@LongLiveCHIEF
Copy link
Contributor

There are many reasons that I believe we should remove support for this feature. They are:

  • these are secrets, and allowing them to be set without any sort of encryption encourages the bad practice of committing the same keys that allow someone to gain access to your database
  • Changes in the admin section of the UI can cause this file to be changed by gitlab itself, and having this file's content managed by puppet would undo those changes during puppet agent runs
  • For HA supported installations, there is no guarantee that the secrets file will be updated on each application role simultaneously, causing users to experience 500 errors

Implementing this change would be a backwards incompatible change, requiring a major version increment.

However, I feel that it is everyone's best interest to remove this feature. I don't want anyone to use it as it currently stands without being fully aware of the security risks that come along with using it.

@LongLiveCHIEF LongLiveCHIEF added backwards-incompatible needs-feedback Further information is requested labels Apr 9, 2018
@LongLiveCHIEF
Copy link
Contributor Author

We could add support for mounting/managing the /etc/gitlab directory as a whole, somewhat mitigating the loss of this feature, but IMO that' probably better suited for an implementing users' profile class.

@jkroepke
Copy link
Contributor

Just to know, I'm using this feature right now. For security reasons like ansible-vault puppet eyaml is good enough.

Anyway moving this feature to a own profile covers my case. Thanks for the information.

@LongLiveCHIEF
Copy link
Contributor Author

The more I've run into this, the more I realize it's a critical decision point in the GitLab HA configuration process. Ensuring your application_role nodes have the same secrets is critical, but it's very difficult to achieve in an automated fashion.

There's no real way to NFS mount these, since you'd have to mount /etc/gitlab. This would of course hinder the ability to configure each node according to their role.

The other option presents difficulty as well, since the puppet agent isn't guaranteed to propagate changes to your secrets to all application nodes at once.

This may actually be best suited via a bolt task.

I've also found you have to run a reconfigure after applying any change to secrets, and this could get complicated in an HA environment where there may be upgrade jobs going on in the background to accommodate zero-downtime upgrades. (In fact at the moment, it appears that all the documentation for updates in HA only mention zero-downtime upgrade processes and no other alternatives.)

@LongLiveCHIEF
Copy link
Contributor Author

I finally came to a decision on this one, mostly due to some updates I worked out to the documentation of Gitlab HA.

So, some secrets are 'shared secrets', and others aren't meant to managed across nodes. The shared secrets can and should be set in /etc/gitlab/gitlab.rb, and support for those actually already exist via gitlab_shell and gitlab_rails parameters. See: https://docs.gitlab.com/ee/administration/high_availability/gitlab.html#extra-configuration-for-additional-gitlab-application-servers

Managing the content in the /etc/gitlab/gitlab-secrets.json file via puppet just winds up creating a never-ending conflict between chef and puppet, which winds up causing your puppet agent to run gitlab reconfigure every time it runs, because chef goes through and changes the secrets file after it's been changed by puppet.

So the end decision is: remove support for setting gitlab-secrets.json file, and determine where/if documentation in this module should be updated to reflect the removal of this support in favor of setting the necesarry secrets in the gitlab.rb file using existing parameters.

@LongLiveCHIEF
Copy link
Contributor Author

One other thing i found is that there are issues with setting the non-shared secrets in this manner, since it winds up double-escaping the line breaks in the certificate values in the json strings, causing failure of the puppet parser when you try to set secrets that aren't meant to be managed anyways.

LongLiveCHIEF added a commit to LongLiveCHIEF/puppet-gitlab that referenced this issue Jun 21, 2018
@LongLiveCHIEF LongLiveCHIEF added this to the v3.0.0 milestone Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatible needs-feedback Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants