action #106365
closedopenQA Project - coordination #105624: [saga][epic] Reconsider how openQA handles secrets
Improve security for OSD worker credentials broke Gitlab CI/CD deploy of salt in OSD size:M
Added by osukup about 2 years ago. Updated over 1 year ago.
0%
Description
Motivation¶
https://progress.opensuse.org/issues/105405 .. changed visibility of salt-pillars-openqa broke deploy stage of CI
Acceptance criteria¶
- AC1: Working salt-states+salt-pillars pipelines in gitlab
- AC2: salt-pillars repo stays non-public
Suggestions¶
- Try out deploy tokens on OSD to fetch the git repo
Updated by osukup about 2 years ago
From CI log:
Your branch is up to date with 'origin/master'.
nothing to commit, working tree clean
+ git fetch origin
fatal: could not read Username for 'https://gitlab.suse.de': No such device or address
Cleaning up project directory and file based variables
Updated by nicksinger about 2 years ago
- Status changed from New to In Progress
- Assignee set to nicksinger
I think this could be fixed by using ssh instead of https here: https://gitlab.suse.de/openqa/salt-states-openqa/-/blob/master/deploy.yml#L26 - I have to look into that and what user/key we could use for this purpose.
Updated by nicksinger about 2 years ago
An even simpler workaround is to use "Access Tokens" for the repository. I created one and could successfully clone the repo with
git clone https://osd_deployment_ci:<SECRET_TOKEN>@gitlab.suse.de/openqa/salt-pillars-openqa.git
will implement this change now into the CI
Updated by nicksinger about 2 years ago
- Status changed from In Progress to Feedback
Updated by nicksinger about 2 years ago
Also a follow-up for the tests we have in place: https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645 - I wonder how we could handle the fact that every fork needs a variable now to be tested successfully (see https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645#note_380972). Any clue how this should work? Will the variable be "copied" if I create a new fork of this repository?
Updated by livdywan about 2 years ago
nicksinger wrote:
Also a follow-up for the tests we have in place: https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645 - I wonder how we could handle the fact that every fork needs a variable now to be tested successfully (see https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645#note_380972). Any clue how this should work? Will the variable be "copied" if I create a new fork of this repository?
I thought that was going to be answered as part of #105405. Regardless I wanted to test it just now, but I get errors whenever I try to fork...
Updated by nicksinger about 2 years ago
cdywan wrote:
nicksinger wrote:
Also a follow-up for the tests we have in place: https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645 - I wonder how we could handle the fact that every fork needs a variable now to be tested successfully (see https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/645#note_380972). Any clue how this should work? Will the variable be "copied" if I create a new fork of this repository?
I thought that was going to be answered as part of #105405. Regardless I wanted to test it just now, but I get errors whenever I try to fork...
Well this ticket here is about fixing the CI which would include having working tests again. But I think as long as the deployment on origin/master works we can also consider the working tests as part of #105405 - fine for me
Updated by livdywan about 2 years ago
- Subject changed from Improve security for OSD worker credentials broke Gitlab CI/CD deploy of salt in OSD to Improve security for OSD worker credentials broke Gitlab CI/CD deploy of salt in OSD size:M
- Description updated (diff)
Updated by nicksinger about 2 years ago
- Status changed from Feedback to Resolved
I made several small adjustments to the pipeline to properly expand the variables:
https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/646
https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/647
https://gitlab.suse.de/openqa/salt-states-openqa/-/merge_requests/648
and with these changes I ended up in a similar situation: https://gitlab.suse.de/openqa/salt-states-openqa/-/jobs/835376#L75
Today I realized that the pipeline does not do a full clone while deploying on existing infrastructure therefore I had to adjust the remote on OSD itself with:
git remote remove origin
git remote add origin https://osd_deployment_ci:<SECTET_TOKEN>@gitlab.suse.de/openqa/salt-pillars-openqa.git
git branch --set-upstream-to=origin/master master
With this the pipeline was finally able to deploy the newest pillars again: https://gitlab.suse.de/openqa/salt-states-openqa/-/jobs/835418#L76
The fact that now MRs can't execute tests (because the secret token is missing for forks) I would try to address in https://progress.opensuse.org/issues/105405
Updated by nicksinger about 2 years ago
I think we can't do that much to make the deployment "successful" right now. I created a follow-up ticket to improve our salt states to not have this issue any more in the future: https://progress.opensuse.org/issues/106666
Updated by okurz over 1 year ago
- Tags changed from salt, gitlab, infrastructure to salt, gitlab, infra