Project

General

Profile

Actions

action #106365

closed

openQA Project (public) - 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 almost 3 years ago. Updated about 2 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
-
Start date:
2022-01-25
Due date:
% Done:

0%

Estimated time:

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
Actions #2

Updated by osukup almost 3 years ago

  • Private changed from Yes to No
Actions #3

Updated by osukup almost 3 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
Actions #4

Updated by nicksinger almost 3 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.

Actions #5

Updated by nicksinger almost 3 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

Actions #6

Updated by nicksinger almost 3 years ago

  • Status changed from In Progress to Feedback
Actions #7

Updated by nicksinger almost 3 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?

Actions #8

Updated by livdywan almost 3 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...

Actions #9

Updated by nicksinger almost 3 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

Actions #10

Updated by livdywan almost 3 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)
Actions #11

Updated by nicksinger almost 3 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

Actions #12

Updated by nicksinger almost 3 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

Actions #13

Updated by okurz almost 3 years ago

ok, that's great. Thanks.

Actions #14

Updated by okurz almost 3 years ago

  • Parent task set to #105405
Actions #15

Updated by okurz about 2 years ago

  • Tags changed from salt, gitlab, infrastructure to salt, gitlab, infra
Actions

Also available in: Atom PDF