Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Gitlab support: RepoClient #2655

Merged
merged 7 commits into from
Mar 13, 2023
Merged

✨ Gitlab support: RepoClient #2655

merged 7 commits into from
Mar 13, 2023

Conversation

raghavkaul
Copy link
Contributor

What kind of change does this PR introduce?

This PR adds tests for GitLab support in Scorecard. Some work remains: Some tests require a GitLab test organization, or test repos, or other scaffolding.

What is the current behavior?

This work builds off @N8BWert's work in #2280, but updates the branch with main.

Which issue(s) this PR fixes

Fixes #2266.

Does this PR introduce a user-facing change?

If a user is running scorecard on a GitHub repository that includes the string "gitlab." in the title scorecard will now think that repository is actually a GitLab project, however this is an incredibly rare edge case.

Release Notes

Repositories that include "gitlab." in their title will be assumed to be gitlab projects.

Link to previous PR:
#2265 (comment)

laurentsimon
laurentsimon approved these changes Feb 13, 2023
View reviewed changes
checker/client.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 16, 2023 •
edited
Loading

Codecov Report

Merging #2655 (533dc3e) into main (5625dda) will decrease coverage by 0.20%.
The diff coverage is 28.57%.

❗ Current head 533dc3e differs from pull request most recent head eb0d15b. Consider uploading reports for the commit eb0d15b to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2655      +/-   ##
==========================================
- Coverage   49.86%   49.66%   -0.20%     
==========================================
  Files         156      156              
  Lines       11576    11643      +67     
==========================================
+ Hits         5772     5783      +11     
- Misses       5454     5505      +51     
- Partials      350      355       +5

@github-actions
Copy link

Stale pull request message

@raghavkaul raghavkaul mentioned this pull request Mar 1, 2023
Closed
azeemshaikh38
azeemshaikh38 requested changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

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

Thanks @raghavkaul. Could you please split this PR into smaller ones? Maybe one PR per check? There are 50 changed files with 160 commits in this PR. That's a lot of code to review reasonably.

@raghavkaul raghavkaul temporarily deployed to integration-test March 9, 2023 17:55 — with GitHub Actions Inactive
@raghavkaul raghavkaul changed the title 🌱 Gitlab support: Tests ✨ Gitlab support: RepoClient Mar 9, 2023
@raghavkaul raghavkaul mentioned this pull request Mar 9, 2023
Closed
@raghavkaul
Add make targets and E2E test target for GitLab only
c612630 
Signed-off-by: Raghav Kaul <raghavkaul@google.com>
@raghavkaul raghavkaul temporarily deployed to integration-test March 9, 2023 18:27 — with GitHub Actions Inactive
@raghavkaul
Add GitLab support to RepoClient
533dc3e 
Signed-off-by: Raghav Kaul <raghavkaul@google.com>
@raghavkaul raghavkaul temporarily deployed to integration-test March 9, 2023 18:58 — with GitHub Actions Inactive
azeemshaikh38
azeemshaikh38 approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

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

Looks good overall to submit. Left some minor comments.

.github/workflows/integration.yml Show resolved Hide resolved
Makefile Show resolved Hide resolved
checker/client.go Outdated Show resolved Hide resolved
clients/repo.go Show resolved Hide resolved
clients/gitlabrepo/client.go Show resolved Hide resolved
clients/gitlabrepo/client.go Show resolved Hide resolved
raghavkaul and others added 4 commits March 10, 2023 20:47
@raghavkaul
Build
8c33301 
* Make target for e2e-gitlab-token
* Only run Gitlab tests in CI that don't require a token

Signed-off-by: Raghav Kaul <raghavkaul@google.com>
@raghavkaul
Add tests
57298ba 
Signed-off-by: Raghav Kaul <raghavkaul@google.com>
@raghavkaul
Remove spurious printf
cea0526 
Signed-off-by: Raghav Kaul <raghavkaul@google.com>
@spencerschrock @raghavkaul
🐛 Check OSS Fuzz build file for Fuzzing check (ossf#2719)
09680de 
* Check OSS-Fuzz using project list

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Use clients.RepoClient interface to perform the new OSS Fuzz check

Signed-off-by: Spencer Schrock <sschrock@google.com>

* wip: add eager client for better repeated lookup of projects

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Split lazy and eager behavior into different implementations.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Add tests and benchmarks

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Switch to always parsing JSON to determine if a project is present. The other approach of looking for a substring match would lead to false positives.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Add eager constructor to surface status file errors sooner.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Switch existing users to new OSS Fuzz client

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Mark old method as deprecated in the godoc

Signed-off-by: Spencer Schrock <sschrock@google.com>

* remove unused comment.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Use new OSS Fuzz client in e2e test.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* fix typo.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Fix potential path bug with test server.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Force include the two JSON files which were being ignored by .gitignore

Signed-off-by: Spencer Schrock <sschrock@google.com>

* trim the status json file

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>
@raghavkaul raghavkaul temporarily deployed to integration-test March 10, 2023 21:00 — with GitHub Actions Inactive
@raghavkaul raghavkaul temporarily deployed to integration-test March 13, 2023 15:02 — with GitHub Actions Inactive
@raghavkaul raghavkaul merged commit 110e352 into ossf:main Mar 13, 2023
@raghavkaul raghavkaul deleted the gitlab_tests branch March 13, 2023 15:14
azeemshaikh38 added a commit to azeemshaikh38/scorecard that referenced this pull request Mar 14, 2023
@azeemshaikh38
Squashed commit of the following:
36f35fc 
commit 00da7a8be965c09d044f978d6b9eafee1350bd30
Author: Azeem Shaikh <azeemshaikh38@gmail.com>
Date:   Tue Mar 14 23:07:19 2023 +0000

    Pr comments

commit 1127dd9
Merge: 274448f 23bd295
Author: Azeem Shaikh <azeemshaikh38@gmail.com>
Date:   Wed Mar 15 04:23:32 2023 +0530

    Merge branch 'main' into go-git

commit 274448f
Author: Azeem Shaikh <azeemshaikh38@gmail.com>
Date:   Tue Mar 14 22:52:30 2023 +0000

    Initial implementation of go-git client

    Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>

commit 23bd295
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Mar 14 20:28:41 2023 +0000

    :seedling: Bump github/codeql-action from 2.2.4 to 2.2.6 (ossf#2741)

commit fc026ef
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Mar 14 17:04:31 2023 +0000

    :seedling: Bump github.com/google/ko from 0.12.0 to 0.13.0 in /tools (ossf#2742)

commit 2e04214
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Mar 14 14:02:34 2023 +0000

    :seedling: Bump tj-actions/changed-files from 35.6.2 to 35.7.0

    Bumps [tj-actions/changed-files](https://github.com/tj-actions/changed-files) from 35.6.2 to 35.7.0.
    - [Release notes](https://github.com/tj-actions/changed-files/releases)
    - [Changelog](https://github.com/tj-actions/changed-files/blob/main/HISTORY.md)
    - [Commits](tj-actions/changed-files@5ce975c...bd376fb)

    ---
    updated-dependencies:
    - dependency-name: tj-actions/changed-files
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

commit e36b590
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Mar 14 08:59:20 2023 -0500

    :seedling: Bump actions/cache from 3.3.0 to 3.3.1 (ossf#2740)

    Bumps [actions/cache](https://github.com/actions/cache) from 3.3.0 to 3.3.1.
    - [Release notes](https://github.com/actions/cache/releases)
    - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
    - [Commits](actions/cache@940f3d7...88522ab)

    ---
    updated-dependencies:
    - dependency-name: actions/cache
      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>

commit 6ff94eb
Author: Gabriela Gutierrez <gabigutierrez@google.com>
Date:   Mon Mar 13 19:42:37 2023 +0000

    :bug: Handle editable pip installs (ossf#2731)

    * fix: Handle editable pip install

    Editable pip installs (-e) should be considered secure if the package is installed from a local source or a remote source (VCS install) but pinned by commit hash. To keep the behaviour we have for normal pip installs, we need to guarantee the package dependencies are pinned by hash too. For normal pip installs, we verify that by using --require-hashes flag. Unfortunately, --require-hashes flag is not compatible with editable installs, so we use --no-deps flag to verify the dependencies are not installed since we can't verify if they are pinned.

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Editable pip install in GHA

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Editable pip install in Dockerfile

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Editable pip install in shell script

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * fix: Code complexity increase

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * fix: Simplify boolean return

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * docs: Add pip editable install references in comments

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * fix: Handle multiple packages in editable pip install

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Multi editable pip install in GHA

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Multi editable pip install in Dockerfile

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    * test: Multi editable pip install in shell script

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

    ---------

    Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>
    Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>

commit 110e352
Author: raghavkaul <8695110+raghavkaul@users.noreply.github.com>
Date:   Mon Mar 13 11:13:50 2023 -0400

    ✨ Gitlab support: RepoClient (ossf#2655)

    * Add make targets and E2E test target for GitLab only

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>

    * Add GitLab support to RepoClient

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>

    * Build

    * Make target for e2e-gitlab-token
    * Only run Gitlab tests in CI that don't require a token

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>

    * Add tests

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>

    * Remove spurious printf

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>

    * 🐛 Check OSS Fuzz build file for Fuzzing check (ossf#2719)

    * Check OSS-Fuzz using project list

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Use clients.RepoClient interface to perform the new OSS Fuzz check

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * wip: add eager client for better repeated lookup of projects

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Split lazy and eager behavior into different implementations.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Add tests and benchmarks

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Switch to always parsing JSON to determine if a project is present. The other approach of looking for a substring match would lead to false positives.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Add eager constructor to surface status file errors sooner.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Switch existing users to new OSS Fuzz client

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Mark old method as deprecated in the godoc

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * remove unused comment.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Use new OSS Fuzz client in e2e test.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * fix typo.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Fix potential path bug with test server.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Force include the two JSON files which were being ignored by .gitignore

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * trim the status json file

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    ---------

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    ---------

    Signed-off-by: Raghav Kaul <raghavkaul@google.com>
    Signed-off-by: Spencer Schrock <sschrock@google.com>
    Co-authored-by: Spencer Schrock <sschrock@google.com>

commit 5625dda
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Sat Mar 11 17:14:42 2023 +0000

    :seedling: Bump github.com/onsi/ginkgo/v2 from 2.8.3 to 2.9.0 in /tools

    Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.8.3 to 2.9.0.
    - [Release notes](https://github.com/onsi/ginkgo/releases)
    - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md)
    - [Commits](onsi/ginkgo@v2.8.3...v2.9.0)

    ---
    updated-dependencies:
    - dependency-name: github.com/onsi/ginkgo/v2
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

commit d591e38
Author: Spencer Schrock <sschrock@google.com>
Date:   Fri Mar 10 16:02:05 2023 -0800

    🌱  Add RepoClient re-use E2E tests. (ossf#2625)

    * Add e2e test for re-used repoclient.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Improve diff logging

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Skip scorecard e2e test during unit tests.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    * Fix linter.

    Signed-off-by: Spencer Schrock <sschrock@google.com>

    ---------

    Signed-off-by: Spencer Schrock <sschrock@google.com>

commit a7e81bb
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Fri Mar 10 08:20:28 2023 -0600

    :seedling: Bump actions/cache from 3.2.6 to 3.3.0 (ossf#2738)

    Bumps [actions/cache](https://github.com/actions/cache) from 3.2.6 to 3.3.0.
    - [Release notes](https://github.com/actions/cache/releases)
    - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
    - [Commits](actions/cache@69d9d44...940f3d7)

    ---
    updated-dependencies:
    - dependency-name: actions/cache
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@pnacht pnacht mentioned this pull request Mar 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgangerlm kgangerlm kgangerlm left review comments

@azeemshaikh38 azeemshaikh38 azeemshaikh38 approved these changes

@laurentsimon laurentsimon laurentsimon approved these changes

@justaugustus justaugustus Awaiting requested review from justaugustus

@naveensrinivasan naveensrinivasan Awaiting requested review from naveensrinivasan

@spencerschrock spencerschrock Awaiting requested review from spencerschrock

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for GitLab Projects
5 participants
@raghavkaul @azeemshaikh38 @spencerschrock @laurentsimon @kgangerlm