Skip to content

Commit

Permalink
🌱 cron: generalize config and create optional values for scorecard an…
Browse files Browse the repository at this point in the history 
…d criticality (2/n) (#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>
  • Loading branch information
@spencerschrock @azeemshaikh38
spencerschrock and azeemshaikh38 authored Sep 12, 2022
1 parent 9e269b8 commit bde0ae1
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 60 deletions.
131 changes: 88 additions & 43 deletions cron/internal/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,11 @@ const (
shardSize string = "SCORECARD_SHARD_SIZE"
webhookURL string = "SCORECARD_WEBHOOK_URL"
metricExporter string = "SCORECARD_METRIC_EXPORTER"
ciiDataBucketURL string = "SCORECARD_CII_DATA_BUCKET_URL"
blacklistedChecks string = "SCORECARD_BLACKLISTED_CHECKS"
bigqueryTable string = "SCORECARD_BIGQUERY_TABLE"
resultDataBucketURL string = "SCORECARD_DATA_BUCKET_URL"
apiResultsBucketURL string = "SCORECARD_API_RESULTS_BUCKET_URL"
inputBucketURL string = "SCORECARD_INPUT_BUCKET_URL"
inputBucketPrefix string = "SCORECARD_INPUT_BUCKET_PREFIX"
// Raw results.
rawBigqueryTable string = "RAW_SCORECARD_BIGQUERY_TABLE"
rawResultDataBucketURL string = "RAW_SCORECARD_DATA_BUCKET_URL"
)

var (
Expand All @@ -66,24 +61,19 @@ var (

//nolint:govet
type config struct {
ProjectID string `yaml:"project-id"`
ResultDataBucketURL string `yaml:"result-data-bucket-url"`
RequestTopicURL string `yaml:"request-topic-url"`
RequestSubscriptionURL string `yaml:"request-subscription-url"`
BigQueryDataset string `yaml:"bigquery-dataset"`
BigQueryTable string `yaml:"bigquery-table"`
CompletionThreshold float32 `yaml:"completion-threshold"`
WebhookURL string `yaml:"webhook-url"`
CIIDataBucketURL string `yaml:"cii-data-bucket-url"`
BlacklistedChecks string `yaml:"blacklisted-checks"`
MetricExporter string `yaml:"metric-exporter"`
ShardSize int `yaml:"shard-size"`
// Raw results.
RawResultDataBucketURL string `yaml:"raw-result-data-bucket-url"`
RawBigQueryTable string `yaml:"raw-bigquery-table"`
APIResultsBucketURL string `yaml:"api-results-bucket-url"`
InputBucketURL string `yaml:"input-bucket-url"`
InputBucketPrefix string `yaml:"input-bucket-prefix"`
ProjectID string `yaml:"project-id"`
ResultDataBucketURL string `yaml:"result-data-bucket-url"`
RequestTopicURL string `yaml:"request-topic-url"`
RequestSubscriptionURL string `yaml:"request-subscription-url"`
BigQueryDataset string `yaml:"bigquery-dataset"`
BigQueryTable string `yaml:"bigquery-table"`
CompletionThreshold float32 `yaml:"completion-threshold"`
WebhookURL string `yaml:"webhook-url"`
MetricExporter string `yaml:"metric-exporter"`
ShardSize int `yaml:"shard-size"`
InputBucketURL string `yaml:"input-bucket-url"`
InputBucketPrefix string `yaml:"input-bucket-prefix"`
AdditionalParams map[string]map[string]string `yaml:"additional-params"`
}

func getParsedConfigFromFile(byteValue []byte) (config, error) {
Expand All @@ -95,17 +85,21 @@ func getParsedConfigFromFile(byteValue []byte) (config, error) {
return ret, nil
}

func getConfigValue(envVar string, byteValue []byte, fieldName string) (reflect.Value, error) {
if val, present := os.LookupEnv(envVar); present {
return reflect.ValueOf(val), nil
}
func getReflectedValueFromConfig(byteValue []byte, fieldName string) (reflect.Value, error) {
parsedConfig, err := getParsedConfigFromFile(byteValue)
if err != nil {
return reflect.ValueOf(parsedConfig), fmt.Errorf("error parsing config file: %w", err)
}
return reflect.ValueOf(parsedConfig).FieldByName(fieldName), nil
}

func getConfigValue(envVar string, byteValue []byte, fieldName string) (reflect.Value, error) {
if val, present := os.LookupEnv(envVar); present {
return reflect.ValueOf(val), nil
}
return getReflectedValueFromConfig(byteValue, fieldName)
}

func getStringConfigValue(envVar string, byteValue []byte, fieldName, configName string) (string, error) {
value, err := getConfigValue(envVar, byteValue, fieldName)
if err != nil {
Expand Down Expand Up @@ -154,6 +148,53 @@ func getFloat64ConfigValue(envVar string, byteValue []byte, fieldName, configNam
}
}

func envVarName(subMapName, subKeyName string) string {
base := fmt.Sprintf("%s_%s", subMapName, subKeyName)
underscored := strings.ReplaceAll(base, "-", "_")
return strings.ToUpper(underscored)
}

// getMapConfigValue returns a map from a nested yaml file. The values can be overridden if an env variable
// is set which corresponds to the name of the nested map and nested key. For example, the baz-qux value in
// the returned map can be overridden if FOO_BAR_BAZ_QUX is set.
// In the example below, "additional-params" is the fieldName, and "foo-bar" is the subMapName:
//
// additional-params:
// foo-bar:
// baz-qux:
func getMapConfigValue(byteValue []byte, fieldName, configName, subMapName string) (map[string]string, error) {
value, err := getReflectedValueFromConfig(byteValue, fieldName)
if err != nil {
return map[string]string{}, fmt.Errorf("error getting config value %s: %w", configName, err)
}
if value.Kind() != reflect.Map {
return map[string]string{}, fmt.Errorf("%w: %s, %s", ErrorValueConversion, value.Type().Name(), configName)
}
subMap := value.MapIndex(reflect.ValueOf(subMapName))
if subMap.Kind() != reflect.Map {
return map[string]string{}, fmt.Errorf("%w: %s, %s", ErrorValueConversion, value.Type().Name(), configName)
}
ret := map[string]string{}
iter := subMap.MapRange()
for iter.Next() {
subKey := iter.Key().String()
val := iter.Value().String()
if v, present := os.LookupEnv(envVarName(subMapName, subKey)); present {
val = v
}
ret[subKey] = val
}
return ret, nil
}

func getScorecardParam(key string) (string, error) {
s, err := GetScorecardValues()
if err != nil {
return "", err
}
return s[key], nil
}

// GetProjectID returns the cloud projectID for the cron job.
func GetProjectID() (string, error) {
return getStringConfigValue(projectID, configYAML, "ProjectID", "project-id")
Expand Down Expand Up @@ -191,14 +232,12 @@ func GetCompletionThreshold() (float64, error) {

// GetRawBigQueryTable returns the table name to transfer cron job results.
func GetRawBigQueryTable() (string, error) {
return getStringConfigValue(rawBigqueryTable, configYAML,
"RawBigQueryTable", "raw-bigquery-table")
return getScorecardParam("raw-bigquery-table")
}

// GetRawResultDataBucketURL returns the bucketURL for storing cron job's raw results.
func GetRawResultDataBucketURL() (string, error) {
return getStringConfigValue(rawResultDataBucketURL, configYAML,
"RawResultDataBucketURL", "raw-result-data-bucket-url")
return getScorecardParam("raw-result-data-bucket-url")
}

// GetShardSize returns the shard_size for the cron job.
Expand All @@ -217,20 +256,13 @@ func GetWebhookURL() (string, error) {

// GetCIIDataBucketURL returns the bucket URL where CII data is stored.
func GetCIIDataBucketURL() (string, error) {
url, err := getStringConfigValue(ciiDataBucketURL, configYAML, "CIIDataBucketURL", "cii-data-bucket-url")
if err != nil && !errors.Is(err, ErrorEmptyConfigValue) {
return url, err
}
return url, nil
return getScorecardParam("cii-data-bucket-url")
}

// GetBlacklistedChecks returns a list of checks which are not to be run.
func GetBlacklistedChecks() ([]string, error) {
checks, err := getStringConfigValue(blacklistedChecks, configYAML, "BlacklistedChecks", "blacklisted-checks")
if err != nil && !errors.Is(err, ErrorEmptyConfigValue) {
return nil, err
}
return strings.Split(checks, ","), nil
checks, err := getScorecardParam("blacklisted-checks")
return strings.Split(checks, ","), err
}

// GetMetricExporter returns the opencensus exporter type.
Expand All @@ -240,8 +272,7 @@ func GetMetricExporter() (string, error) {

// GetAPIResultsBucketURL returns the bucket URL for storing cron job results.
func GetAPIResultsBucketURL() (string, error) {
return getStringConfigValue(apiResultsBucketURL, configYAML,
"APIResultsBucketURL", "api-results-bucket-url")
return getScorecardParam("api-results-bucket-url")
}

// GetInputBucketURL() returns the bucket URL for input files.
Expand All @@ -257,3 +288,17 @@ func GetInputBucketPrefix() (string, error) {
}
return prefix, nil
}

func GetAdditionalParams(subMapName string) (map[string]string, error) {
return getMapConfigValue(configYAML, "AdditionalParams", "additional-params", subMapName)
}

// GetScorecardValues() returns a map of key, value pairs containing additional, scorecard specific values.
func GetScorecardValues() (map[string]string, error) {
return GetAdditionalParams("scorecard")
}

// GetCriticalityValues() returns a map of key, value pairs containing additional, criticality specific values.
func GetCriticalityValues() (map[string]string, error) {
return GetAdditionalParams("criticality")
}
23 changes: 14 additions & 9 deletions cron/internal/config/config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,22 @@ bigquery-table: scorecard-v2
completion-threshold: 0.99
shard-size: 10
webhook-url:
cii-data-bucket-url: gs://ossf-scorecard-cii-data
# TODO: Temporarily remove SAST and CI-Tests which require lot of GitHub API tokens.
# TODO(#859): Re-add Contributors after fixing inconsistencies.
blacklisted-checks: CI-Tests,Contributors
metric-exporter: stackdriver
result-data-bucket-url: gs://ossf-scorecard-data2
# Raw results.
raw-result-data-bucket-url: gs://ossf-scorecard-rawdata
raw-bigquery-table: scorecard-rawdata
# API results bucket
api-results-bucket-url: gs://ossf-scorecard-cron-results
input-bucket-url: gs://ossf-scorecard-input-projects
# Can be used to specify directories within a bucket. Can be empty.
input-bucket-prefix:

additional-params:
criticality:

scorecard:
# API results bucket
api-results-bucket-url: gs://ossf-scorecard-cron-results
# TODO: Temporarily remove SAST and CI-Tests which require lot of GitHub API tokens.
# TODO(#859): Re-add Contributors after fixing inconsistencies.
blacklisted-checks: CI-Tests,Contributors
cii-data-bucket-url: gs://ossf-scorecard-cii-data
# Raw results.
raw-bigquery-table: scorecard-rawdata
raw-result-data-bucket-url: gs://ossf-scorecard-rawdata
71 changes: 65 additions & 6 deletions cron/internal/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ const (
prodInputBucketPrefix = ""
)

var (
prodScorecardParams = map[string]string{
"api-results-bucket-url": prodAPIBucketURL,
"blacklisted-checks": prodBlacklistedChecks,
"cii-data-bucket-url": prodCIIDataBucket,
"raw-bigquery-table": prodRawBigQueryTable,
"raw-result-data-bucket-url": prodRawBucket,
}
prodCriticalityParams map[string]string = nil
prodAdditionalParams = map[string]map[string]string{
"scorecard": prodScorecardParams,
"criticality": prodCriticalityParams,
}
)

func getByteValueFromFile(filename string) ([]byte, error) {
if filename == "" {
return nil, nil
Expand All @@ -55,9 +70,9 @@ func getByteValueFromFile(filename string) ([]byte, error) {
func TestYAMLParsing(t *testing.T) {
t.Parallel()
testcases := []struct {
expectedConfig config
name string
filename string
expectedConfig config
}{
{
name: "validate",
Expand All @@ -71,15 +86,11 @@ func TestYAMLParsing(t *testing.T) {
BigQueryTable: prodBigQueryTable,
CompletionThreshold: prodCompletionThreshold,
WebhookURL: prodWebhookURL,
CIIDataBucketURL: prodCIIDataBucket,
BlacklistedChecks: prodBlacklistedChecks,
ShardSize: prodShardSize,
MetricExporter: prodMetricExporter,
RawResultDataBucketURL: prodRawBucket,
RawBigQueryTable: prodRawBigQueryTable,
APIResultsBucketURL: prodAPIBucketURL,
InputBucketURL: prodInputBucketURL,
InputBucketPrefix: prodInputBucketPrefix,
AdditionalParams: prodAdditionalParams,
},
},

Expand All @@ -101,6 +112,21 @@ func TestYAMLParsing(t *testing.T) {
ShardSize: 250,
},
},
{
name: "optional map values",
filename: "testdata/optional_maps.yaml",
expectedConfig: config{
AdditionalParams: map[string]map[string]string{
"criticality": {
"empty-test": "",
},
"scorecard": {
"cii-data-bucket-url": "gs://ossf-scorecard-cii-data",
"result-data-bucket-url": "gs://ossf-scorecard-data2",
},
},
},
},
}
for _, testcase := range testcases {
testcase := testcase
Expand Down Expand Up @@ -404,3 +430,36 @@ func TestInputBucket(t *testing.T) {
})
}
}

func TestEnvVarName(t *testing.T) {
t.Parallel()
tests := []struct {
name string
mapName string
subKey string
want string
}{
{
name: "basic",
mapName: "foo",
subKey: "bar",
want: "FOO_BAR",
},
{
name: "with dashes",
mapName: "foo-bar",
subKey: "baz-qux",
want: "FOO_BAR_BAZ_QUX",
},
}
for _, testcase := range tests {
testcase := testcase
t.Run(testcase.name, func(t *testing.T) {
t.Parallel()
got := envVarName(testcase.mapName, testcase.subKey)
if got != testcase.want {
t.Errorf("test failed: expected - %s, got = %s", testcase.want, got)
}
})
}
}
21 changes: 21 additions & 0 deletions cron/internal/config/testdata/optional_maps.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Copyright 2022 Security Scorecard Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

additional-params:
scorecard:
cii-data-bucket-url: gs://ossf-scorecard-cii-data
result-data-bucket-url: gs://ossf-scorecard-data2

criticality:
empty-test:
2 changes: 1 addition & 1 deletion cron/k8s/controller.release.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ spec:
- name: SCORECARD_DATA_BUCKET_URL
value: "gs://ossf-scorecard-data-releasetest2"
# Raw results.
- name: RAW_SCORECARD_DATA_BUCKET_URL
- name: SCORECARD_RAW_RESULT_DATA_BUCKET_URL
value: "gs://ossf-scorecard-rawdata-releasetest"
- name: SCORECARD_SHARD_SIZE
value: "5"
Expand Down
2 changes: 1 addition & 1 deletion cron/k8s/worker.release.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
env:
- name: SCORECARD_DATA_BUCKET_URL
value: "gs://ossf-scorecard-data-releasetest2"
- name: RAW_SCORECARD_DATA_BUCKET_URL
- name: SCORECARD_RAW_RESULT_DATA_BUCKET_URL
value: "gs://ossf-scorecard-rawdata-releasetest"
- name: SCORECARD_REQUEST_SUBSCRIPTION_URL
value: "gcppubsub://projects/openssf/subscriptions/scorecard-batch-worker-releasetest"
Expand Down

0 comments on commit bde0ae1

Please sign in to comment.