Skip to content

WIP -- How would we hide the display on /tests/$id#settings pages? -- Add support for configurable secrets variables#5691

Draft
okurz wants to merge 2 commits intoos-autoinst:masterfrom
okurz:feature/hide_custom_secrets_162086
Draft

WIP -- How would we hide the display on /tests/$id#settings pages? -- Add support for configurable secrets variables#5691
okurz wants to merge 2 commits intoos-autoinst:masterfrom
okurz:feature/hide_custom_secrets_162086

Conversation

@okurz
Copy link
Copy Markdown
Member

@okurz okurz commented Jun 11, 2024

@okurz okurz marked this pull request as draft June 11, 2024 14:25
sub redact_settings ($vars) {
return {map { $_ !~ qr/(^_SECRET_|_PASSWORD)/ ? ($_ => $vars->{$_}) : ($_ => '[redacted]') } keys %$vars};
my $hide_re = '^_SECRET_|_PASSWORD';
$hide_re .= "|$vars->{_HIDE_MATCH_RE}" if $vars->{_HIDE_MATCH_RE};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make it a separate regex and also handle errors in case _HIDE_MATCH_RE doesn't contain a valid regex (in which case our default regexes should at least still be applied but some kind of user visible error would be great as well).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be a bit more verbose with the variable name:

Suggested change
$hide_re .= "|$vars->{_HIDE_MATCH_RE}" if $vars->{_HIDE_MATCH_RE};
$hide_re .= "|$vars->{_HIDE_SECRETS_REGEX}" if $vars->{_HIDE_SECRETS_REGEX};

(This is clearer then the abbreviation "re". Considering we are hiding secrets here I would replace "match" with "secrets".)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both suggestions covered including the fallback to ignore the additional custom stuff and still apply the default. I am still doing a concatenation to form the final regex. With more procedural code we could also use if /$default_re/ || /$custom_re/ but IMHO it wouldn't be better. WDYT?

@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch from 17ac889 to 3b7630f Compare June 11, 2024 16:21
@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch from 1418f00 to 3b7630f Compare June 25, 2024 13:23
@stale
Copy link
Copy Markdown

stale bot commented Jun 27, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 27, 2025
@okurz okurz removed the stale label Jun 27, 2025
@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch 2 times, most recently from 2cf6b78 to 171a33d Compare March 23, 2026 14:02
@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch 2 times, most recently from 11d7f5e to 31d7f10 Compare March 31, 2026 21:39
@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch from 31d7f10 to 90b4683 Compare April 1, 2026 11:53
@okurz okurz force-pushed the feature/hide_custom_secrets_162086 branch from 90b4683 to ec07446 Compare April 13, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants