Fix dependency on ApplicationSetting stubs for `let_it_be`

What does this MR do and why?

What happened?

In Saturday 2024-11-23 08:36 UTC - `gitlab-org/git... (gitlab-org/quality/engineering-productivity/master-broken-incidents#9437 - closed) • Fabio Pitino • 17.7 master was broken due to ApplicationSetting#ci_job_token_signing_key being not stubbed. As a temporary fix to unblock master, in !173635 (merged) we made the feature flag to be off by default in specs.

We can only stub application settings inside each example, in a before(:each) block. However, the use of let_it_be that called the generation of a CI_JOB_TOKEN was failing because let_it_be is executed in a before(:all) and the application setting ci_job_token_signin_key was not stubbed yet, returning nil.

The particular case that triggered the CI_JOB_TOKEN generation is the Ci::Build factory with :trace_live trait. In fact, the :trace_live trait calls Ci::Build#hide_secrets which in turns generates the CI_JOB_TOKEN variable.

How we are fixing it?

The way we are fixing this in this MR is to introduce a private method Trace#unsafe_set that is only used in the factory and which doesn't call Ci::Build#hide_secrets since the default trace value used in specs is safe to set.

We are also undo the temporary changes introduced in !173635 (merged).

Alternative approaches tried

In Draft: Move CI job token signing key to secrets... (!173637 - closed) I tried an alternative approach as suggested by Stan. However, if we configure the signing key in secrets.yml it would be harder to implement secrets rotation later. This change also would require changes to Omnibus and Charts as well as follow-ups in subsequent releases to remove the database column.

References

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Fabio Pitino

Merge request reports

Loading