Skip to content

CORENET-7116: Fix security job - exclude vendor and upgrade to SHA256#3019

Open
jluhrsen wants to merge 2 commits into
openshift:masterfrom
jluhrsen:CORENET-7116
Open

CORENET-7116: Fix security job - exclude vendor and upgrade to SHA256#3019
jluhrsen wants to merge 2 commits into
openshift:masterfrom
jluhrsen:CORENET-7116

Conversation

@jluhrsen

@jluhrsen jluhrsen commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Snyk scanner flags SHA1 and MD5 usage as weak hash algorithms even though they're only used for config change detection (not cryptographic purposes). Upgrading to SHA256 eliminates the scanner warnings without changing functionality.

Changes:

  • pkg/network/ovn_kubernetes.go: SHA1 → SHA256 for ConfigMap checksums
  • pkg/util/k8s/unstructured.go: MD5 → SHA256 for object checksums

Related: CORENET-7116

Summary by CodeRabbit

  • Chores
    • Added Snyk configuration to exclude the vendor directory from scans.
    • Switched hashing algorithms to SHA-256 across networking and utility components for stronger, consistent digests.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 2, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@jluhrsen: This pull request references CORENET-7116 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Snyk scanner flags SHA1 and MD5 usage as weak hash algorithms even though they're only used for config change detection (not cryptographic purposes). Upgrading to SHA256 eliminates the scanner warnings without changing functionality.

Changes:

  • pkg/network/ovn_kubernetes.go: SHA1 → SHA256 for ConfigMap checksums
  • pkg/util/k8s/unstructured.go: MD5 → SHA256 for object checksums

Related: CORENET-7116

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c4dbbb72-13ab-47f0-8430-38afb9b64bed

📥 Commits

Reviewing files that changed from the base of the PR and between 90f0b42 and 1aac2a7.

📒 Files selected for processing (3)
  • .snyk
  • pkg/network/ovn_kubernetes.go
  • pkg/util/k8s/unstructured.go
✅ Files skipped from review due to trivial changes (1)
  • .snyk
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/network/ovn_kubernetes.go
  • pkg/util/k8s/unstructured.go

Walkthrough

This pull request migrates hashing implementations to SHA-256 (utility CalculateHash and OVN-Kubernetes config-hash initializers) and adds a .snyk configuration to exclude vendor/** from Snyk scanning.

Changes

Hash Algorithm Migration

Layer / File(s) Summary
Utility hash function upgrade to SHA-256
pkg/util/k8s/unstructured.go
CalculateHash function replaces MD5 with SHA-256 hashing, updating imports and digest computation.
OVN Kubernetes config hash migration to SHA-256
pkg/network/ovn_kubernetes.go
Three config-hash initializers (rendered ConfigMap data, no-overlay node outboundSNAT, control-plane/bgpManagedConfig) updated from SHA-1 to SHA-256 and import changed.
Snyk vendor directory exclusion
.snyk
Configuration file added to exclude vendor/** directory from Snyk Code scanning.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main changes: upgrading from weak hash algorithms (SHA1/MD5) to SHA256 and excluding the vendor directory from Snyk scanning.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR does not modify test files and codebase uses standard Go testing, not Ginkgo. Check is not applicable.
Test Structure And Quality ✅ Passed No Ginkgo test files are included in this PR. Changes are limited to production code (.snyk config, hash algorithm upgrades in ovn_kubernetes.go and unstructured.go). Check is not applicable.
Microshift Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests. The custom check for MicroShift test compatibility is not applicable here since only utility code and configuration files are modified.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests are added in this PR. The changes are to configuration files and non-test utility code (hash algorithm upgrades), so the SNO test compatibility check does not apply.
Topology-Aware Scheduling Compatibility ✅ Passed PR only modifies hash algorithms (SHA1/MD5→SHA256) and Snyk config; introduces no deployment manifests, controllers, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed This PR modifies only library code (hash algorithm upgrades in pkg/network and pkg/util), adding no stdout writes to process-level code or test infrastructure.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests added. PR only modifies .snyk config and production code (SHA1/MD5 to SHA256 upgrades).
No-Weak-Crypto ✅ Passed PR removes weak crypto (SHA1/MD5) and replaces with SHA256. No MD5, SHA1, DES, RC4, 3DES, Blowfish, or ECB usage found in codebase. Only secure algorithms used.
Container-Privileges ✅ Passed PR contains no container/K8s manifests. Changes are: .snyk configuration file, and Go source code files updating hash algorithms from SHA1/MD5 to SHA256. No privileged container settings found.
No-Sensitive-Data-In-Logs ✅ Passed PR changes only hash algorithms (SHA-1→SHA-256, MD5→SHA-256) for internal config checksums; hash values are not logged and no sensitive data exposure in logs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/Masterminds/semver@v1.5.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/Masterminds/sprig/v3@v3.2.3: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/containernetworking/cni@v0.8.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/ghodss/yaml@v1.0.1-0.20190212211648-25d852aebe32: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/go-bindata/go-bindata@v3.1.2+incompatible: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/onsi/gomega@v1.39.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/ope

... [truncated 17357 characters] ...

red in go.mod, but not marked as explicit in vendor/modules.txt\n\tk8s.io/gengo/v2@v2.0.0-20251215205346-5ee0d033ba5b: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tk8s.io/kms@v0.35.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tk8s.io/kube-aggregator@v0.35.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tsigs.k8s.io/randfill@v1.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tsigs.k8s.io/structured-merge-diff/v6@v6.3.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n"


Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested a review from arghosh93 June 2, 2026 21:07
@openshift-ci

openshift-ci Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jluhrsen
Once this PR has been reviewed and has the lgtm label, please assign tssurya for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot requested a review from jcaamano June 2, 2026 21:07

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.snyk:
- Around line 1-6: Update the broken Snyk docs link in the .snyk file: remove or
replace the invalid URL
"https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test"
with the current Snyk docs URL (or delete the reference entirely) so it no
longer returns 404; keep the other valid URL
("https://docs.snyk.io/snyk-cli/commands/ignore" or its redirect) as-is and
ensure the comment text still accurately reflects the referenced docs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ea285fe1-523e-4de8-a4af-e5fd613c5012

📥 Commits

Reviewing files that changed from the base of the PR and between 7d4c17a and 9c4f49a.

📒 Files selected for processing (3)
  • .snyk
  • pkg/network/ovn_kubernetes.go
  • pkg/util/k8s/unstructured.go

Comment thread .snyk
Comment on lines +1 to +6
# References:
# https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test
# https://docs.snyk.io/snyk-cli/commands/ignore
exclude:
global:
- 'vendor/**' # Upstream dependencies (not project code)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that the Snyk documentation URLs are valid and return HTTP 200

echo "Checking first documentation URL..."
curl -s -o /dev/null -w "%{http_code}" "https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test"

echo "Checking second documentation URL..."
curl -s -o /dev/null -w "%{http_code}" "https://docs.snyk.io/snyk-cli/commands/ignore"

Repository: openshift/cluster-network-operator

Length of output: 157


🏁 Script executed:

#!/bin/bash
set -euo pipefail

check_url () {
  local url="$1"
  echo "== $url =="
  echo "-- HEAD (no redirect) --"
  curl -sS -o /dev/null -w "HTTP:%{http_code}\n" -I "$url" || true

  echo "-- GET follow redirects --"
  # Print final HTTP code and the final effective URL
  final_code=$(curl -sSL -o /dev/null -w "%{http_code}" "$url" || true)
  final_url=$(curl -sSL -o /dev/null -w "%{url_effective}" "$url" || true)
  echo "HTTP_AFTER_REDIRECTS:${final_code}"
  echo "URL_EFFECTIVE:${final_url}"
  echo
}

check_url "https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test"
check_url "https://docs.snyk.io/snyk-cli/commands/ignore"

Repository: openshift/cluster-network-operator

Length of output: 685


Fix broken Snyk docs reference in .snyk

  • https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test returns 404 (link needs updating/removal).
  • https://docs.snyk.io/snyk-cli/commands/ignore redirects to https://docs.snyk.io/developer-tools/snyk-cli/snyk-cli/commands/ignore and works.

Otherwise, excluding vendor/** looks appropriate for scanning third-party dependencies.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.snyk around lines 1 - 6, Update the broken Snyk docs link in the .snyk
file: remove or replace the invalid URL
"https://docs.snyk.io/scan-applications/snyk-code/using-snyk-code-from-the-cli/excluding-directories-and-files-from-the-snyk-code-cli-test"
with the current Snyk docs URL (or delete the reference entirely) so it no
longer returns 404; keep the other valid URL
("https://docs.snyk.io/snyk-cli/commands/ignore" or its redirect) as-is and
ensure the comment text still accurately reflects the referenced docs.

@jluhrsen jluhrsen changed the title CORENET-7116: Replace weak hash algorithms with SHA256 CORENET-7116: Fix security job - exclude vendor and upgrade to SHA256 Jun 2, 2026
@jluhrsen

jluhrsen commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

/test 5.0-upgrade-from-stable-4.22-e2e-azure-ovn-upgrade
/test e2e-aws-ovn-upgrade
/test e2e-azure-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6-ipsec

jluhrsen and others added 2 commits June 9, 2026 14:07
Snyk was scanning vendored upstream dependencies which are
not under our direct control.

Related: CORENET-7116

Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Snyk scanner flags SHA1 and MD5 as weak hash algorithms
even though we only use them for config change detection.
Upgrading to SHA256 eliminates the warnings.

Related: CORENET-7116

Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jluhrsen

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@jluhrsen: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants