ROX-26908: set operator fips-compliant:true#13219
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test ? |
|
@davdhacs: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
DetailsIn response to this:
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. |
|
/test ocp-next-candidate-fips-qa-e2e-tests ocp-4-17-fips-qa-e2e-tests gke-operator-e2e-tests ocp-4-12-operator-e2e-tests |
|
Images are ready for the commit at c39457c. To use with deploy scripts, first |
davdhacs
changed the title
davdhacs
changed the title
msugakov
left a comment
msugakov
left a comment
There was a problem hiding this comment.
LGTM
Please: reference JIRA ticket (so that we can recover context later) and get approval from one of the Install team colleagues.
davdhacs
changed the title
davdhacs
added
the
backport release-4.6
davdhacs
changed the title
vladbologa
left a comment
vladbologa
left a comment
There was a problem hiding this comment.
I'm not very familiar with the topic, but I think that for fips compliance we need to:
- use a fips compliant base image
- compile with
CGO_ENABLED=1so that the binaries use the OpenSSL libs from the base image
I think that downstream and Konflux already do that, but quickly going through our code, the upstream might not, e.g. see https://github.com/stackrox/stackrox/blob/master/operator/Dockerfile#L52
Is that a problem? cc @msugakov @porridge @mclasmeier
|
Declaring FIPS upstream but not supporting it is fine, in my opinion.
FWIW, UBI8 has fips-certified openssl (we use only that). CentOS Stream most certainly doesn't. It would be impossible to be fully FIPS upstream but nobody cares, I believe. FIPS is important for the commercial product. |
I agree with you, I just wonder if we should actually mention anywhere that the upstream build is marked as fips compliant when it's actually not. |
vladbologa
left a comment
vladbologa
left a comment
There was a problem hiding this comment.
LGTM.
I was a bit concerned that we're also marking the upstream build as fips compliant when it's not (see my other comments and Misha's reply). We could at least mention this somewhere (even the description of this PR would be ok for me).
I understand. I neglected to add the context of https://gitlab.cee.redhat.com/stackrox/rhacs-midstream/-/merge_requests/3608?diff_id=7164818 where I argued for setting this only on the builds where it is true. But the same conclusion was reached there and I understand the simplicity of making it true everywhere because it does not matter in the non-fips build (operator not published). I'll add a comment in the template file and update this PR description. Is there somewhere else it could be useful to have a note about this? |
@vladbologa I'm sorry to not give more context about this also. We have confirmed the downstream images are fips-compliant on 4.5.x (base image has fips-compliant openssl, and our golang and c binaries are using it, and the non-openssl crypto functions used in our golang are not prohibited ones (https://docs.google.com/document/d/1uEV5rhmuwwVgrRfgXIEf9dNQLSiK78t1P8-h8aKpjhU/edit?tab=t.0#heading=h.61lj3tho6qxr)). More details are recorded in https://docs.google.com/document/d/1KcjlevcMF4DLi-KROru7tuUUIb4wSk5e6bnXpU8HZxA/edit?tab=t.0 and the parent ticket for this change: https://issues.redhat.com/browse/ROX-24283. |
operator/bundle/manifests/rhacs-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
davdhacs
removed
the
backport release-4.6
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #13219 +/- ##
==========================================
- Coverage 48.55% 48.55% -0.01%
==========================================
Files 2467 2467
Lines 177781 177781
==========================================
- Hits 86317 86314 -3
- Misses 84537 84538 +1
- Partials 6927 6929 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This reverts commit 1f8b5d6.
|
/retest |
|
Confirmed in https://issues.redhat.com/browse/ROX-26908 that "No additional checks required from Install team's perspective." |
davdhacs
added
the
backport release-4.6
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-4.5 release-4.5
# Navigate to the new working tree
cd .worktrees/backport-release-4.5
# Create a new branch
git switch --create backport-13219-to-release-4.5
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d0c43be2b60411325886e897cdc7ffb8829ceed9
# Push it to GitHub
git push --set-upstream origin backport-13219-to-release-4.5
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-4.5Then, create a pull request where the |
4 participants
Description
Setting fips-compliant operator feature to true.
This is not technically true for upstream. But we do not publish the upstream operator, and it is true for the published operator (downstream (CPaaS and Konflux)).
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!