Project

General

Profile

action #106365

openQA 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 5 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
High
Assignee:
Target version:
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

History

#2 Updated by osukup 5 months ago

  • Private changed from Yes to No

#3 Updated by osukup 5 months 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

#4 Updated by nicksinger 5 months 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.

#5 Updated by nicksinger 5 months 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

#6 Updated by nicksinger 5 months ago

  • Status changed from In Progress to Feedback

#7 Updated by nicksinger 5 months 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?

#8 Updated by cdywan 5 months 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...

#9 Updated by nicksinger 5 months 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

#10 Updated by cdywan 5 months 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)

#11 Updated by nicksinger 5 months 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

#12 Updated by nicksinger 5 months 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

#13 Updated by okurz 5 months ago

ok, that's great. Thanks.

#14 Updated by okurz 4 months ago

  • Parent task set to #105405

Also available in: Atom PDF