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.

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

🌱 cron: generalize config and create optional values for scorecard and criticality (2/n) #2254

Merged
merged 7 commits into from
Sep 12, 2022

Conversation

spencerschrock
Copy link
Contributor

What kind of change does this PR introduce?

Part of a series of refactors to generalize the cron infrastructure to enable re-use by the ossf criticality score project

What is the current behavior?

Every config value was a top level yaml field

What is the new behavior (if this is a feature change)?**

Only fields which are general enough to be re-used both by scorecard and criticality_score remain as top level fields. Anything specific to either program have been moved into nested fields called scorecard and criticality. These will be parsed as key, value pairs (both strings).

The values can still be overwritten by environment variables. Consider the example below, which can be overridded by setting the environment variable SCORECARD_EXAMPLE_FIELD:

scorecard:
  example-field: foo
  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock
Copy link
Contributor Author

@calebbrown @oliverchang fyi

The map can be fetched with config.GetCriticalityValues(). And the PR/code documents how to override the default values with environment variables. When you add your config values, some unit tests will need to change, but that should more or less be it.

@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #2254 (f02f298) into main (9e269b8) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2254   +/-   ##
=======================================
  Coverage   44.45%   44.45%           
=======================================
  Files          95       95           
  Lines        7987     7987           
=======================================
  Hits         3551     3551           
  Misses       4173     4173           
  Partials      263      263           

@github-actions
Copy link

Integration tests success for
[d53dbea]
(https://github.com/ossf/scorecard/actions/runs/3039621154)

cron/internal/config/config.go Outdated Show resolved Hide resolved
@azeemshaikh38
Copy link
Contributor

Thanks! LGTM with comment

cron/internal/config/config.go Outdated Show resolved Hide resolved
cron/internal/config/config.go Outdated Show resolved Hide resolved
Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock spencerschrock temporarily deployed to integration-test September 12, 2022 21:14 Inactive
@github-actions
Copy link

Integration tests success for
[3a6f554]
(https://github.com/ossf/scorecard/actions/runs/3040477779)

@azeemshaikh38 azeemshaikh38 enabled auto-merge (squash) September 12, 2022 23:12
@azeemshaikh38 azeemshaikh38 merged commit bde0ae1 into ossf:main Sep 12, 2022
@spencerschrock spencerschrock deleted the cron-config-refactor branch September 14, 2022 16:39
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Sep 23, 2022
…d criticality (2/n) (ossf#2254)

* Add map logic to yaml config.

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

* Add scorecard yaml test

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

* Separate general config values from scorecard specific values.

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

* Add criticality values to config.

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

* Add test to confirm empty string behavior.

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

* Combine scorecard and criticality values under AdditionalParams.

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

Signed-off-by: Spencer Schrock <sschrock@google.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
…d criticality (2/n) (ossf#2254)

* Add map logic to yaml config.

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

* Add scorecard yaml test

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

* Separate general config values from scorecard specific values.

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

* Add criticality values to config.

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

* Add test to confirm empty string behavior.

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

* Combine scorecard and criticality values under AdditionalParams.

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

Signed-off-by: Spencer Schrock <sschrock@google.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Signed-off-by: nathaniel.wert <nathaniel.wert@kudelskisecurity.com>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
…d criticality (2/n) (ossf#2254)

* Add map logic to yaml config.

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

* Add scorecard yaml test

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

* Separate general config values from scorecard specific values.

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

* Add criticality values to config.

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

* Add test to confirm empty string behavior.

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

* Combine scorecard and criticality values under AdditionalParams.

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

Signed-off-by: Spencer Schrock <sschrock@google.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Signed-off-by: nathaniel.wert <nathaniel.wert@kudelskisecurity.com>
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