Skip to content

Fix secrets detection build step to improve branch detection#9320

Merged
aduth merged 6 commits intomainfrom
aduth-fix-secrets-detection
Oct 5, 2023
Merged

Fix secrets detection build step to improve branch detection#9320
aduth merged 6 commits intomainfrom
aduth-fix-secrets-detection

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 5, 2023

🛠 Summary of changes

Fixes failing builds caused by git failures in the script.

[git] fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.

Borrows liberally from changelog script logic for git branching:

identity-idp/.gitlab-ci.yml

Lines 140 to 152 in 00e4701

- |
if [ "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" == "main" ]
then
git fetch origin --quiet
./scripts/changelog_check.rb -b origin/"${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}" -s origin/"${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME}"
elif [ "$CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_NAME" == "main" ]
then
git fetch origin --quiet
./scripts/changelog_check.rb -b origin/"${CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_NAME}" -s origin/"${CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_NAME}"
else
echo "Skipping because this is not a PR or is not targeting main"
exit 0
fi

Discussion: https://gsa-tts.slack.com/archives/C0NGESUN5/p1696511447586139?thread_ts=1696289942.572699&cid=C0NGESUN5

Previous PR: #9296

📜 Testing Plan

Build should pass.

interruptible: true
variables:
BRANCH_TAGGING_STRING: ""
BRANCH_TAGGING_STRING: ''
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a few line changes like this, are they needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a few line changes like this, are they needed?

Not strictly necessary, they're from my Prettier auto-formatter.

We apply Prettier already to a good chunk of the YAML files in this repository and I'd feel semi-strongly that it should be applied / enforced here as well, but absent the tooling for that, I think the occasional manual "sync-up" is okay?

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing!

changelog: Internal, Continuous Integration, Add secret scanning job
@aduth aduth merged commit c79ebd0 into main Oct 5, 2023
@aduth aduth deleted the aduth-fix-secrets-detection branch October 5, 2023 15:37
@jmdembe jmdembe mentioned this pull request Oct 10, 2023
jmdembe added a commit that referenced this pull request Oct 10, 2023
* Fix secrets detection build step to improve branch detection (#9320)

* Empty commit

* Fetch prior to secrets analyze

* Better main detection

* s/and/&&/

* Fix "if" syntax

* Add changelog

changelog: Internal, Continuous Integration, Add secret scanning job

* Retire the ProfileMigrationJob (#9322)

The ProfileMigrationJob has migrated all of the Profile records that needed migrating so it can be removed

[skip changelog]

* Bump libphonenumber-js from 1.10.45 to 1.10.46 (#9325)

Bumps [libphonenumber-js](https://gitlab.com/catamphetamine/libphonenumber-js) from 1.10.45 to 1.10.46.
- [Changelog](https://gitlab.com/catamphetamine/libphonenumber-js/blob/master/CHANGELOG.md)
- [Commits](https://gitlab.com/catamphetamine/libphonenumber-js/compare/v1.10.45...v1.10.46)

---
updated-dependencies:
- dependency-name: libphonenumber-js
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix alert paragraph rendering semantics, redundant styling (#9317)

* Remove unnecessary text_tag overrides

changelog: Bug Fixes, Alerts, Fix HTML semantics for some alerts

* Remove redundant margin reset on alert paragraph

* Gitignore .bak files (#9324)

* Gitignore .bak files

Avoid accidentally checking in stray backup files.

[skip changelog]

* Ignore files with .bak extension

---------

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Bump libphonenumber-js from 1.10.46 to 1.10.47 (#9332)

Bumps [libphonenumber-js](https://gitlab.com/catamphetamine/libphonenumber-js) from 1.10.46 to 1.10.47.
- [Changelog](https://gitlab.com/catamphetamine/libphonenumber-js/blob/master/CHANGELOG.md)
- [Commits](https://gitlab.com/catamphetamine/libphonenumber-js/compare/v1.10.46...v1.10.47)

---
updated-dependencies:
- dependency-name: libphonenumber-js
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update guidance for frontend error logging (#9330)

changelog: Internal, Documentation, Update guidance for frontend error logging

* Rename ial2_consent_given (3/3) (#9288)

Follow on to #9287, removes all references to / support for ial2_consent_given. Should not be merged until that PR is deployed.

[skip changelog]

* Exclude certain paths from secrets detection (#9337)

There are false positives identified when running the Gitlab secret detector locally.

[skip changelog]

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>
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.

3 participants