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

ROX-12577 Scanner: load Istio dump #955

Merged
merged 7 commits into from
Oct 17, 2022
Merged

Conversation

daynewlee
Copy link
Contributor

@daynewlee daynewlee commented Sep 30, 2022

In this pr, we want to make sure Scanner can load Istio dump, detect changes in Istio dump in run time and in CI.

@ghost
Copy link

ghost commented Sep 30, 2022

Images are ready for the commit at 406c7b5.

To use the images, use the tag 2.26-37-g406c7b5e0b.

@daynewlee daynewlee added the generate-dumps-on-pr Generates the image based on dumps from the PR label Sep 30, 2022
@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch 5 times, most recently from e742532 to 272d78a Compare October 6, 2022 04:17
@daynewlee daynewlee requested a review from RTann October 6, 2022 16:40
@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch 4 times, most recently from 2d653fd to ad8ad7d Compare October 10, 2022 15:42
@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch from ad8ad7d to f0c41b1 Compare October 10, 2022 15:43
cmd/updater/diffdumps/cmd.go Outdated Show resolved Hide resolved
continue
}
if err := generateK8sDiff(k8sSubDir, baseFiles[name], headF); err != nil {
if err := generateK8sDiff(subDir, baseFiles[name], headF); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

It should not be generateK8sDiff here. This function does not seem to be generic to different component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is wrong. I will fix this part.

@@ -99,10 +107,10 @@ func generateK8sDiffs(outputDir string, baseZipR *zip.ReadCloser, headZipR *zip.
}

// Only look at YAML files in the k8s/ folder.
Copy link
Contributor

Choose a reason for hiding this comment

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

K8s?

@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch from e2f85dd to 248b000 Compare October 12, 2022 13:58
@daynewlee daynewlee requested a review from c-du October 12, 2022 16:44
"github.com/stackrox/istio-cves/types"
)

// LoadYAMLFileFromReader loads the Kubernetes CVE feed from the given io.Reader.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Istio vulnerability feed

if err != nil {
return types.Vuln{}, errors.Wrap(err, "reading YAML contents")
}
var schema types.Vuln
Copy link
Collaborator

Choose a reason for hiding this comment

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

schema => vuln

image/scanner/dump/genesis_manifests.json Show resolved Hide resolved
@@ -1990,7 +1990,7 @@ var testCases = []testCase{
},
AddedBy: "sha256:36e8e9714b9a509fae9e515ff16237928c3d809f5ae228b14d2f7d7605c02623",
Location: "jars/jackson-databind-2.9.10.4.jar",
FixedBy: "2.12.6.1",
FixedBy: "2.14.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should really be done in a separate PR, but it's fine for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was using another pr but the CI always timed out. Meanwhile I didn't remove this change from this pr.

@@ -98,12 +148,18 @@ func generateK8sDiffs(outputDir string, baseZipR *zip.ReadCloser, headZipR *zip.
continue
}

// Only look at YAML files in the k8s/ folder.
if filepath.Dir(name) != vulndump.K8sDirName || filepath.Ext(name) != ".yaml" {
// Only look at YAML files in the k8s/ or istio/ folder.
Copy link
Collaborator

Choose a reason for hiding this comment

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

in case we add another for some reason, let's just say "Only look at YAML files in the given dirName directory."

continue
}
if err := generateK8sDiff(k8sSubDir, baseFiles[name], headF); err != nil {
return errors.Wrapf(err, "generating Kubernetes diff for file %q", headF.Name)
if outputDir == vulndump.K8sDirName {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of doing this check, maybe we pass in a generateDiffFunc to this function since they both have the same function signature

defer utils.IgnoreError(outF.Close)

if !reflect.DeepEqual(baseIstioDump, istioDump) {
log.Infof("Kubernetes CVE file %q is in the diff", headF.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Istio

if !reflect.DeepEqual(baseIstioDump, istioDump) {
log.Infof("Kubernetes CVE file %q is in the diff", headF.Name)
if _, err := io.Copy(outF, headReader); err != nil {
return errors.Wrap(err, "copying Kubernetes CVE file")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Istio vulnerability

@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch from 0ccf242 to 30882af Compare October 14, 2022 19:16
@daynewlee daynewlee force-pushed the personal/yli3/addIstioDump branch from 1ed9d95 to f8fd461 Compare October 14, 2022 19:44
@daynewlee daynewlee requested a review from RTann October 14, 2022 21:57
@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2022

@daynewlee: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests f8fd461 link false /test e2e-tests
ci/prow/slim-e2e-tests f8fd461 link false /test slim-e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Copy link
Collaborator

@RTann RTann left a comment

Choose a reason for hiding this comment

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

very minor nits. Did you also confirm the vulns are stored in the scanner image as expected?

@@ -69,9 +73,57 @@ func generateK8sDiff(outputDir string, baseF, headF *zip.File) error {
return nil
}

func generateIstioDiffHelper(outputDir string, baseF, headF *zip.File) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: generateIstioDiff

if err := generateK8sDiff(k8sSubDir, baseFiles[name], headF); err != nil {
return errors.Wrapf(err, "generating Kubernetes diff for file %q", headF.Name)
if err := generateDiffs(subDir, baseFiles[name], headF); err != nil {
return errors.Wrapf(err, "generating Istio diff for file %q", headF.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove Istio, as it's not clear if this is k8s or Istio at this time

@daynewlee
Copy link
Contributor Author

very minor nits. Did you also confirm the vulns are stored in the scanner image as expected?

yli3-mac:scanner yili$ kubectl exec -it scanner-6849b95c44-zzvz5 -n stackrox -- /bin/bash
bash-4.4$ ls
THIRD_PARTY_NOTICES  ci		    generate-junit-reports.sh  import-additional-cas  lib.sh	  mnt		   push-as-manifest-list.sh  run		srv	       usr
bin		     dev	    genesis_manifests.json     istio_definitions      lib64	  nvd_definitions  repo2cpe		     save-dir-contents	sys	       var
boot		     entrypoint.sh  go-get-version.sh	       k8s_definitions	      lost+found  opt		   restore-all-dir-contents  sbin		tmp
cert		     etc	    home		       lib		      media	  proc		   root			     scanner		trust-root-ca
bash-4.4$ cd istio_definitions/
bash-4.4$ ls
ISTIO-SECURITY-2019-001.yaml  ISTIO-SECURITY-2019-006.yaml  ISTIO-SECURITY-2020-004.yaml  ISTIO-SECURITY-2020-009.yaml	ISTIO-SECURITY-2021-006.yaml  ISTIO-SECURITY-2022-003.yaml
ISTIO-SECURITY-2019-002.yaml  ISTIO-SECURITY-2019-007.yaml  ISTIO-SECURITY-2020-005.yaml  ISTIO-SECURITY-2020-010.yaml	ISTIO-SECURITY-2021-007.yaml  ISTIO-SECURITY-2022-004.yaml
ISTIO-SECURITY-2019-003.yaml  ISTIO-SECURITY-2020-001.yaml  ISTIO-SECURITY-2020-006.yaml  ISTIO-SECURITY-2021-001.yaml	ISTIO-SECURITY-2021-008.yaml  ISTIO-SECURITY-2022-005.yaml
ISTIO-SECURITY-2019-004.yaml  ISTIO-SECURITY-2020-002.yaml  ISTIO-SECURITY-2020-007.yaml  ISTIO-SECURITY-2021-003.yaml	ISTIO-SECURITY-2022-001.yaml  ISTIO-SECURITY-2022-006.yaml
ISTIO-SECURITY-2019-005.yaml  ISTIO-SECURITY-2020-003.yaml  ISTIO-SECURITY-2020-008.yaml  ISTIO-SECURITY-2021-005.yaml	ISTIO-SECURITY-2022-002.yaml

@daynewlee daynewlee requested a review from RTann October 17, 2022 18:09
Copy link
Collaborator

@RTann RTann left a comment

Choose a reason for hiding this comment

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

Don't forget to change the PR title :)

@daynewlee daynewlee changed the title Personal/yli3/add istio dump ROX-12577 Scanner: load Istio dump Oct 17, 2022
@daynewlee daynewlee merged commit f1ae85e into master Oct 17, 2022
@daynewlee daynewlee deleted the personal/yli3/addIstioDump branch October 17, 2022 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generate-dumps-on-pr Generates the image based on dumps from the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants