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

internal/pkg/scorecard: access CRManifestOpt correctly with --olm-deployed #1565

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- Fixes an issue that causes Helm RBAC generation to fail when creating new operators with a Kubernetes context configured to connect to an OpenShift cluster. ([#1461](https://github.com/operator-framework/operator-sdk/pull/1461))
- Generated CSV's that include a deployment install strategy will be checked for a reference to `metadata.annotations['olm.targetNamespaces']`, and if one is not found a reference will be added to the `WATCH_NAMESPACE` env var for all containers in the deployment. This is a bug because any other value that references the CSV's namespace is incorrect. ([#1396](https://github.com/operator-framework/operator-sdk/pull/1396))
- Build `-trimpath` was not being respected. `$GOPATH` was not expanding because `exec.Cmd{}` is not executed in a shell environment. ([#1535](https://github.com/operator-framework/operator-sdk/pull/1535))
- Running the [scorecard](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#up) with `--olm-deployed` will now only use the first CR set in either the `cr-manifest` config option or the CSV's `metadata.annotations['alm-examples']` as was intended, and access manifests correctly from the config. ([#1565](https://github.com/operator-framework/operator-sdk/pull/1565))

## v0.8.0

Expand Down
76 changes: 45 additions & 31 deletions internal/pkg/scorecard/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
Expand All @@ -34,6 +33,7 @@ import (
"github.com/ghodss/yaml"
olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
olminstall "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/viper"
Expand Down Expand Up @@ -171,37 +171,51 @@ func runTests() ([]scapiv1alpha1.ScorecardOutput, error) {
return nil, err
}

// Create a temporary CR manifest from metadata if one is not provided.
crJSONStr, ok := csv.ObjectMeta.Annotations["alm-examples"]
if ok && viper.GetString(CRManifestOpt) == "" {
var crs []interface{}
if err = json.Unmarshal([]byte(crJSONStr), &crs); err != nil {
return nil, err
}
// TODO: run scorecard against all CR's in CSV.
cr := crs[0]
crJSONBytes, err := json.Marshal(cr)
if err != nil {
return nil, err
}
crYAMLBytes, err := yaml.JSONToYAML(crJSONBytes)
if err != nil {
return nil, err
}
crFile, err := ioutil.TempFile("", "cr.yaml")
if err != nil {
return nil, err
}
if _, err := crFile.Write(crYAMLBytes); err != nil {
return nil, err
}
viper.Set(CRManifestOpt, crFile.Name())
defer func() {
err := os.Remove(viper.GetString(CRManifestOpt))
logCRMsg := false
if crMans := viper.GetStringSlice(CRManifestOpt); len(crMans) == 0 {
// Create a temporary CR manifest from metadata if one is not provided.
if crJSONStr, ok := csv.ObjectMeta.Annotations["alm-examples"]; ok {
var crs []interface{}
if err = json.Unmarshal([]byte(crJSONStr), &crs); err != nil {
return nil, err
}
// TODO: run scorecard against all CR's in CSV.
cr := crs[0]
logCRMsg = len(crs) > 1
crJSONBytes, err := json.Marshal(cr)
if err != nil {
log.Errorf("Could not delete temporary CR manifest file: (%v)", err)
return nil, err
}
}()
crYAMLBytes, err := yaml.JSONToYAML(crJSONBytes)
if err != nil {
return nil, err
}
crFile, err := ioutil.TempFile("", "*.cr.yaml")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the file *.cr.yaml now?

Copy link
Member Author

Choose a reason for hiding this comment

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

The temp file names were in an annoying format before 😁. This format uses .cr.yaml as a file extension.

if err != nil {
return nil, err
}
if _, err := crFile.Write(crYAMLBytes); err != nil {
return nil, err
}
viper.Set(CRManifestOpt, []string{crFile.Name()})
defer func() {
for _, f := range viper.GetStringSlice(CRManifestOpt) {
if err := os.Remove(f); err != nil {
log.Errorf("Could not delete temporary CR manifest file: (%v)", err)
}
}
}()
} else {
return nil, errors.New("cr-manifest config option must be set if CSV has no metadata.annotations['alm-examples']")
}
} else {
// TODO: run scorecard against all CR's in CSV.
viper.Set(CRManifestOpt, []string{crMans[0]})
logCRMsg = len(crMans) > 1
}
// Let users know that only the first CR is being tested.
if logCRMsg {
log.Infof("The scorecard does not support testing multiple CR's at once when run with --olm-deployed. Testing the first CR %s", viper.GetStringSlice(CRManifestOpt)[0])
}

} else {
Expand Down Expand Up @@ -486,7 +500,7 @@ func configureLogger() error {
}

func validateScorecardFlags() error {
if !viper.GetBool(OlmDeployedOpt) && viper.GetStringSlice(CRManifestOpt) == nil {
if !viper.GetBool(OlmDeployedOpt) && len(viper.GetStringSlice(CRManifestOpt)) == 0 {
return errors.New("cr-manifest config option must be set")
}
if !viper.GetBool(BasicTestsOpt) && !viper.GetBool(OLMTestsOpt) {
Expand Down