🌱 cron: pass config as an argument to binaries (4/n) #2279

spencerschrock · 2022-09-20T18:55:56Z

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

PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

The cron/internal/config/config.yaml file is embedded inside cron binaries, and packaged with the various docker files.
This was not flexible for re-using the cron infrastructure across projects.

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

All cron binaries which interact with the config package now support a config flag.

  --config string
        Location of config file (default "/etc/scorecard/config.yaml")

The config file is now volume mounted for k8s cronjobs at /etc/scorecard/config.yaml

Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

I made a few naming choices, (e.g. storing configs at /etc/scorecard), feel free to nit and I can patch.

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>

codecov · 2022-09-20T20:13:34Z

Codecov Report

Merging #2279 (7be9cff) into main (97df43b) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2279   +/-   ##
=======================================
  Coverage   41.17%   41.17%           
=======================================
  Files         110      110           
  Lines        8774     8774           
=======================================
  Hits         3613     3613           
  Misses       4892     4892           
  Partials      269      269

github-actions · 2022-09-20T20:42:29Z

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

cron/internal/bq/main.go

cron/internal/config/config.go

oliverchang · 2022-09-20T23:54:36Z

@calebbrown FYI

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

github-actions · 2022-09-21T18:11:36Z

Integration tests success for
[6ab5556]
(https://github.com/ossf/scorecard/actions/runs/3099673793)

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

github-actions · 2022-09-22T17:43:32Z

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

…nded to be temporary to help with GKE rollout. Signed-off-by: Spencer Schrock <sschrock@google.com>

github-actions · 2022-09-22T18:34:06Z

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

github-actions · 2022-09-22T18:38:25Z

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

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

github-actions · 2022-09-23T17:54:29Z

Integration tests success for
[7be9cff]
(https://github.com/ossf/scorecard/actions/runs/3114545144)

cron/k8s/controller.yaml

* Explicitly read config file instead of embedding it. Signed-off-by: Spencer Schrock <sschrock@google.com> * Add CLI config arg and ReadConfig() to existing cron binaries. Signed-off-by: Spencer Schrock <sschrock@google.com> * Volume mount config Signed-off-by: Spencer Schrock <sschrock@google.com> * Ignore CLI flag args when reading local filenames in controller. Signed-off-by: Spencer Schrock <sschrock@google.com> * Hide --config in the config package. Signed-off-by: Spencer Schrock <sschrock@google.com> * Add config param to k8s files. Signed-off-by: Spencer Schrock <sschrock@google.com> * Fix test Signed-off-by: Spencer Schrock <sschrock@google.com> * Allow fallback to embedded config if no config is passed as arg. Intended to be temporary to help with GKE rollout. Signed-off-by: Spencer Schrock <sschrock@google.com> Signed-off-by: Spencer Schrock <sschrock@google.com> Signed-off-by: nathaniel.wert <nathaniel.wert@kudelskisecurity.com>

* Explicitly read config file instead of embedding it. Signed-off-by: Spencer Schrock <sschrock@google.com> * Add CLI config arg and ReadConfig() to existing cron binaries. Signed-off-by: Spencer Schrock <sschrock@google.com> * Volume mount config Signed-off-by: Spencer Schrock <sschrock@google.com> * Ignore CLI flag args when reading local filenames in controller. Signed-off-by: Spencer Schrock <sschrock@google.com> * Hide --config in the config package. Signed-off-by: Spencer Schrock <sschrock@google.com> * Add config param to k8s files. Signed-off-by: Spencer Schrock <sschrock@google.com> * Fix test Signed-off-by: Spencer Schrock <sschrock@google.com> * Allow fallback to embedded config if no config is passed as arg. Intended to be temporary to help with GKE rollout. Signed-off-by: Spencer Schrock <sschrock@google.com> Signed-off-by: Spencer Schrock <sschrock@google.com>

spencerschrock added 4 commits September 20, 2022 11:14

Explicitly read config file instead of embedding it.

bf0554e

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

Add CLI config arg and ReadConfig() to existing cron binaries.

6115704

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

Volume mount config

82a206e

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

Ignore CLI flag args when reading local filenames in controller.

bc15232

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

spencerschrock requested review from olivekl, azeemshaikh38, justaugustus, laurentsimon and naveensrinivasan as code owners September 20, 2022 18:55

spencerschrock temporarily deployed to integration-test September 20, 2022 20:04 Inactive

azeemshaikh38 approved these changes Sep 20, 2022

View reviewed changes

cron/internal/bq/main.go Outdated Show resolved Hide resolved

cron/internal/config/config.go Outdated Show resolved Hide resolved

spencerschrock added 4 commits September 21, 2022 09:45

Hide --config in the config package.

fb1c564

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

Add config param to k8s files.

4fbce07

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

Merge branch 'main' into config-arg

120706b

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

Fix test

6ab5556

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

azeemshaikh38 approved these changes Sep 21, 2022

View reviewed changes

spencerschrock temporarily deployed to integration-test September 21, 2022 17:36 Inactive

Merge branch 'main' into config-arg

beca636

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

spencerschrock temporarily deployed to integration-test September 22, 2022 17:21 Inactive

spencerschrock closed this Sep 22, 2022

Allow fallback to embedded config if no config is passed as arg. Inte…

c41a577

…nded to be temporary to help with GKE rollout. Signed-off-by: Spencer Schrock <sschrock@google.com>

spencerschrock reopened this Sep 22, 2022

spencerschrock removed request for naveensrinivasan, justaugustus and laurentsimon September 22, 2022 18:13

spencerschrock requested review from azeemshaikh38 and removed request for olivekl September 22, 2022 18:13

spencerschrock temporarily deployed to integration-test September 22, 2022 18:13 Inactive

spencerschrock temporarily deployed to integration-test September 22, 2022 18:16 Inactive

Merge branch 'main' into config-arg

7be9cff

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

spencerschrock temporarily deployed to integration-test September 23, 2022 17:35 Inactive

azeemshaikh38 approved these changes Sep 23, 2022

View reviewed changes

cron/k8s/controller.yaml Show resolved Hide resolved

azeemshaikh38 enabled auto-merge (squash) September 23, 2022 20:21

azeemshaikh38 merged commit a7a503a into ossf:main Sep 23, 2022

spencerschrock deleted the config-arg branch September 23, 2022 20:43

spencerschrock mentioned this pull request Sep 23, 2022

🌱 cron: Fix k8s yaml errors and document how to prevent them. (5/n) #2298

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 cron: pass config as an argument to binaries (4/n) #2279

🌱 cron: pass config as an argument to binaries (4/n) #2279

spencerschrock commented Sep 20, 2022

codecov bot commented Sep 20, 2022 •

edited

Loading

github-actions bot commented Sep 20, 2022

oliverchang commented Sep 20, 2022

github-actions bot commented Sep 21, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 23, 2022

🌱 cron: pass config as an argument to binaries (4/n) #2279

🌱 cron: pass config as an argument to binaries (4/n) #2279

Conversation

spencerschrock commented Sep 20, 2022

What kind of change does this PR introduce?

What is the current behavior?

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

Which issue(s) this PR fixes

Special notes for your reviewer

Does this PR introduce a user-facing change?

codecov bot commented Sep 20, 2022 • edited Loading

Codecov Report

github-actions bot commented Sep 20, 2022

oliverchang commented Sep 20, 2022

github-actions bot commented Sep 21, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 23, 2022

codecov bot commented Sep 20, 2022 •

edited

Loading