Skip to content

chore: Update lint.yml to reference in-house security lint GH action#3228

Merged
mergify[bot] merged 1 commit intomainfrom
replace-current-security-lint-action-with-ci-action
Mar 25, 2025
Merged

chore: Update lint.yml to reference in-house security lint GH action#3228
mergify[bot] merged 1 commit intomainfrom
replace-current-security-lint-action-with-ci-action

Conversation

@courtneypacheco
Copy link
Contributor

@courtneypacheco courtneypacheco commented Mar 12, 2025 •
edited
Loading

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the
    conventional commits.
  • Changelog updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Functional tests have been added, if necessary.
  • E2E Workflow tests have been added, if necessary.

Background

On Feb 11, 2025, I created an in-house GitHub action called detect-exposed-secrets: #3112

I have since taken the contents of this detect-exposed-secrets action and migrated them to our ci-actions repo here: https://github.com/instructlab/ci-actions/tree/main/actions/detect-exposed-workflow-secrets

During this migration process, I also updated name of the action from detect-exposed-secrets to detect-exposed-workflow-secrets so that the name of the action is accurate. (The original name implied the action might detect any type of exposed secret, when that isn't accurate.)

Proposed Changes

  • Remove the in-house GitHub action that I created ~30 days ago since it now exists in the ci-actions repo
  • Update this repository's lint.yml file to reference the action from the ci-actions repo instead of from this repository.'
  • Pin the version of the reusable action to v0.1.0 so that any updates to the action are not automatically consumed without anyone's knowledge

@mergify mergify bot added CI/CD Affects CI/CD configuration ci-failure PR has at least one CI failure labels Mar 12, 2025
@courtneypacheco courtneypacheco force-pushed the replace-current-security-lint-action-with-ci-action branch 2 times, most recently from c0db389 to 74a5811 Compare March 12, 2025 12:40
@courtneypacheco courtneypacheco mentioned this pull request Mar 12, 2025
Merged
5 tasks
@courtneypacheco courtneypacheco force-pushed the replace-current-security-lint-action-with-ci-action branch 4 times, most recently from 499006d to 738f9ea Compare March 12, 2025 13:11
@mergify mergify bot removed the ci-failure PR has at least one CI failure label Mar 12, 2025
@courtneypacheco courtneypacheco force-pushed the replace-current-security-lint-action-with-ci-action branch from 738f9ea to 1ec7b63 Compare March 18, 2025 16:38
@courtneypacheco courtneypacheco changed the title [DO NOT MERGE] [TESTING] chore: Update lint.yml to reference in-house security lint GH action chore: Update lint.yml to reference in-house security lint GH action Mar 18, 2025
@courtneypacheco courtneypacheco force-pushed the replace-current-security-lint-action-with-ci-action branch from 1ec7b63 to 59b99f2 Compare March 18, 2025 16:41
courtneypacheco
courtneypacheco commented Mar 18, 2025
View reviewed changes
.github/workflows/lint.yml Outdated
fetch-depth: 0
repository: instructlab/ci-actions
path: ci-actions
ref: v0.1.0
Copy link
Contributor Author

@courtneypacheco courtneypacheco Mar 18, 2025

Choose a reason for hiding this comment

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

We have two options for references... We can pin to an actual release, like this one (v0.1.0) or we can pin to a release branch like release-v0.1 to allow for z-stream releases to be automatically consumed. (Note that release-v0.1 is a protected branch and users are not allowed to directly commit to it.)

We cannot use logic liike:

ref: v0.1

to automatically pull in z-stream releases because ref only accepts the following inputs:

  • branch name
  • SHA
  • GitHub tag

In order to reference a GitHub action by a major version (e.g., @v3) or a minor version (e.g., @v3.2), the action needs to be published to the GH marketplace.

Copy link
Member

@nathan-weinberg nathan-weinberg Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Seems the branch option will be more flexible and require less work on either end - I would prefer that

Copy link
Member

@danmcp danmcp Mar 18, 2025

Choose a reason for hiding this comment

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

+1 to the branch option given the alternatives

@courtneypacheco
Update lint.yml to reference in-house GH action from ci-actions
01500fb 
Reference the newly-released GH action, `detect-exposed-workflow-secrets`, which is included in `release-v0.1.0`. Also remove the existing reference from the repo.

Signed-off-by: Courtney Pacheco <6019922+courtneypacheco@users.noreply.github.com>
@courtneypacheco courtneypacheco force-pushed the replace-current-security-lint-action-with-ci-action branch from 59b99f2 to 01500fb Compare March 19, 2025 11:39
@kami619
Copy link
Contributor

kami619 commented Mar 19, 2025 •
edited
Loading

@courtneypacheco I think these changes look good. But I have a question on the base template and the markdown file doesn't point to the new changes which are part of this lint.yaml file. Do we also need to update them ?

@courtneypacheco
Copy link
Contributor Author

courtneypacheco commented Mar 24, 2025

Hey @kami619 yes, I will update the ci-actions repo. I created a GitHub issue to track what you outlined: instructlab/ci-actions#10

@courtneypacheco courtneypacheco marked this pull request as ready for review March 24, 2025 14:24
@mergify mergify bot added the one-approval PR has one approval from a maintainer label Mar 24, 2025
@mergify mergify bot removed the one-approval PR has one approval from a maintainer label Mar 25, 2025
@courtneypacheco courtneypacheco removed the request for review from nathan-weinberg March 25, 2025 13:45
@mergify mergify bot merged commit 3aeff9e into main Mar 25, 2025
30 checks passed
@mergify mergify bot deleted the replace-current-security-lint-action-with-ci-action branch March 25, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nathan-weinberg nathan-weinberg nathan-weinberg left review comments

@ktdreyer ktdreyer ktdreyer approved these changes

+1 more reviewer

@danmcp danmcp danmcp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI/CD Affects CI/CD configuration

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@courtneypacheco @kami619 @danmcp @ktdreyer @nathan-weinberg