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

Duplicate repoName leads to race condition #92

Closed
mhutter opened this issue Aug 7, 2020 · 1 comment · Fixed by #248
Closed

Duplicate repoName leads to race condition #92

mhutter opened this issue Aug 7, 2020 · 1 comment · Fixed by #248
Labels
bug Something isn't working

Comments

@mhutter
Copy link
Contributor

mhutter commented Aug 7, 2020

It is possible to configure the same repoName for multiple GitRepos, which sends lieutenant-operator into an endless deploy-key-reconciliation-loop

Steps to Reproduce the Problem

  1. Create a cluster, using "gitops1" as repoName.
  2. Create a second cluster, also use "gitops1" as a repoName.

Actual Behavior

{"level":"info","ts":1596810672.7987385,"logger":"controller_gitrepo","msg":"Reconciling GitRepo","Request.Namespace":"lieutenant","Request.Name":"c-dawn-lake-5378"}
{"level":"info","ts":1596810675.2224236,"logger":"controller_gitrepo","msg":"keys differed from CRD, keys re-applied to repository","Request.Namespace":"lieutenant","Request.Name":"c-dawn-lake-5378"}
{"level":"info","ts":1596810675.2419648,"logger":"controller_gitrepo","msg":"Reconciling GitRepo","Request.Namespace":"lieutenant","Request.Name":"c-nameless-river-4010"}
{"level":"info","ts":1596810676.9737165,"logger":"controller_gitrepo","msg":"forcing re-creation of key steward","Request.Namespace":"lieutenant","Request.Name":"c-nameless-river-4010"}
{"level":"info","ts":1596810677.6824875,"logger":"controller_gitrepo","msg":"removing key steward; existing on repo but not in CRDs","Request.Namespace":"lieutenant","Request.Name":"c-nameless-river-4010"}
{"level":"info","ts":1596810678.7076182,"logger":"controller_gitrepo","msg":"keys differed from CRD, keys re-applied to repository","Request.Namespace":"lieutenant","Request.Name":"c-nameless-river-4010"}

In my case this actually lead to the situation where NO deploy key was configured on Gitlab.com at all.

Expected Behavior

One of:

  • Lieutenant prevents creation of GitRepos with duplicated repoNames (API? admission hook? maybe possible to configure in the CRD?)
  • Lieutenant supports this configuration and does not fight itself
@mhutter mhutter added the bug Something isn't working label Aug 7, 2020
@mhutter mhutter changed the title Duplicate {{repoName}} leads to race condition Duplicate repoName leads to race condition Aug 7, 2020
@srueg
Copy link
Contributor

srueg commented Aug 11, 2020

Some more thoughts:
The current behavior of adopting/fighting-for existing git repos and adding deploy keys is a security risk: If I create a GitRepo object which points to an existing repo on which the operator's token has access but I don't, the operator will configure the corresponding deploy keys and therefore give me access to the repo.

Possible solution

If a GitRepo object of type auto is created and the referenced URL already exists, an error/warning is logged and the operator switches the type to unmanaged. This would mean it's no longer possible to "adopt" an already existing git repo and manage it via the operator. This would prevent the described privilege escalation.

Possible alternatives

@tobru tobru added this to To do in Project Syn v1.0.0 via automation Sep 7, 2020
@tobru tobru moved this from To do to Selected in Project Syn v1.0.0 Sep 7, 2020
@srueg srueg added this to To do in Project Syn Focus Weeks Oct 2020 via automation Oct 12, 2020
@glrf glrf closed this as completed in #248 Sep 28, 2022
Project Syn v1.0.0 automation moved this from Selected to Done Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

2 participants