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

fix: Removing '-Dcom.redhat.fips=false' from the default JAVA_OPTS #1777

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

ibuziuk
Copy link
Member

@ibuziuk ibuziuk commented Nov 9, 2023

What does this PR do?

Removing '-Dcom.redhat.fips=false' from the default JAVA_OPTS

In the latest Dev Spaces release the latest 6.8.1 version of k8s client is used which is FIPS compatible - fabric8io/kubernetes-client#3582 and there is no need to disable fips via -Dcom.redhat.fips=false on the che-server end.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://issues.redhat.com/browse/CRW-4592

How to test this PR?

  • Provision cluster with FIPS enabled (the easiest way to do it is via clusterbot launch 4.13 azure,fips)
  • Check that the FIPS mode is enabled on the cluster via oc get cm cluster-config-v1 -n kube-system -o json | jq -r '.data' | grep -i "fips" command
Screenshot 2023-11-09 at 16 45 11
  • Install the latest version of Dev Spaces 3.9.0 / 3.9.1 and create a CR with updated JAVA_OPTS without -Dcom.redhat.fips=false
spec:
  components:
    cheServer:
      extraProperties:
        JAVA_OPTS : '-XX:MaxRAMPercentage=85.0'
  • Check the value of the JAVA_OPTS env var in che-server / devspaces pod as well as in the che configmap - it should be -XX:MaxRAMPercentage=85.0
Screenshot 2023-11-09 at 16 56 07
  • start / stop a few workspace and verify that workspace startup has no issues / no errors in the che-server logs

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5d1a58d) 59.53% compared to head (f97d77f) 59.53%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1777   +/-   ##
=======================================
  Coverage   59.53%   59.53%           
=======================================
  Files          71       71           
  Lines        8605     8605           
=======================================
  Hits         5123     5123           
  Misses       3131     3131           
  Partials      351      351           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 14, 2023

@ibuziuk: thanks for the detailed "How to test this PR?" section in description.
The only thing I couldn't find was where to get che-operator image which was built from this PR to test.
Did I miss something? :-)

@tolusha
Copy link
Contributor

tolusha commented Nov 14, 2023

@dmytro-ndp
To test the PR, please

  1. checkout the corresponding branch
  2. run build/scripts/oc-tests/oc-test-operator.sh

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 14, 2023

@dmytro-ndp I believe it is important to test that Dev Spaces 3.8 can not run without -Dcom.redhat.fips=false whereas 3.9 can.

On Dev Spaces 3.8 if this flag is removed, che-server should fail to start with the following error:

Caused by: KeyManagementException: FIPS mode: only SunJSSE TrustManagers may be used
	at java.base/SSLContextImpl.chooseTrustManager(SSLContextImpl.java:133)
	at java.base/SSLContextImpl.engineInit(SSLContextImpl.java:95)
	at java.base/SSLContext.init(SSLContext.java:297)
	at SSLUtils.sslContext(SSLUtils.java:85)
	at HttpClientUtils.applyCommonConfiguration(HttpClientUtils.java:206)

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 14, 2023

@tolusha : thanks for the answer.
I hadn't managed to build che-operator image locally on OS Fedora, because build/scripts/oc-tests/oc-test-operator.sh script execution stuck on +++ make bundle-name CHANNEL=next:

[ndp@fedora che-operator]$ build/scripts/oc-tests/oc-test-operator.sh
++++++ readlink -f build/scripts/oc-tests/oc-test-operator.sh
+++++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts/oc-tests/oc-test-operator.sh
++++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts/oc-tests
+++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts
++ dirname /home/ndp/projects/ibuziuk/che-operator/build
+ export OPERATOR_REPO=/home/ndp/projects/ibuziuk/che-operator
+ OPERATOR_REPO=/home/ndp/projects/ibuziuk/che-operator
+ trap catchFinish EXIT SIGINT
+ runTests
+ . /home/ndp/projects/ibuziuk/che-operator/build/scripts/olm/test-catalog-from-sources.sh --verbose
++ set -e
+++++++ readlink -f /home/ndp/projects/ibuziuk/che-operator/build/scripts/olm/test-catalog-from-sources.sh
++++++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts/olm/test-catalog-from-sources.sh
+++++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts/olm
++++ dirname /home/ndp/projects/ibuziuk/che-operator/build/scripts
+++ dirname /home/ndp/projects/ibuziuk/che-operator/build
++ OPERATOR_REPO=/home/ndp/projects/ibuziuk/che-operator
++ source /home/ndp/projects/ibuziuk/che-operator/build/scripts/oc-tests/oc-common.sh
+++ export NAMESPACE=eclipse-che
+++ NAMESPACE=eclipse-che
+++ export ARTIFACTS_DIR=/tmp/artifacts-che
+++ ARTIFACTS_DIR=/tmp/artifacts-che
+++ export ECLIPSE_CHE_PACKAGE_NAME=eclipse-che
+++ ECLIPSE_CHE_PACKAGE_NAME=eclipse-che
+++ export ECLIPSE_CHE_CATALOG_SOURCE_NAME=eclipse-che
+++ ECLIPSE_CHE_CATALOG_SOURCE_NAME=eclipse-che
+++ export ECLIPSE_CHE_SUBSCRIPTION_NAME=eclipse-che
+++ ECLIPSE_CHE_SUBSCRIPTION_NAME=eclipse-che
++ export ECLIPSE_CHE_ROOT_DIR=/tmp/eclipse-che
++ ECLIPSE_CHE_ROOT_DIR=/tmp/eclipse-che
++ export CATALOG_DIR=/tmp/eclipse-che/olm-catalog/next
++ CATALOG_DIR=/tmp/eclipse-che/olm-catalog/next
++ export BUNDLE_DIR=/tmp/eclipse-che/bundle
++ BUNDLE_DIR=/tmp/eclipse-che/bundle
++ export CA_DIR=/tmp/eclipse-che/certificates
++ CA_DIR=/tmp/eclipse-che/certificates
+++ make bundle-name CHANNEL=next

^C++ export BUNDLE_NAME=
++ BUNDLE_NAME=
+++ catchFinish
+++ local RESULT=0
+++ set +e
+++ chectl server:logs -n eclipse-che -d /tmp/artifacts-che --telemetry off
/home/ndp/projects/ibuziuk/che-operator/build/scripts/oc-tests/oc-common.sh: line 24: chectl: command not found
+++ [[ 0 != \0 ]]
+++ echo '[INFO] Job completed successfully.'
[INFO] Job completed successfully.
+++ rm -rf /home/ndp/projects/ibuziuk/che-operator/tmp
+++ exit 0
+ catchFinish
+ local RESULT=0
+ set +e
+ chectl server:logs -n eclipse-che -d /tmp/artifacts-che --telemetry off
/home/ndp/projects/ibuziuk/che-operator/build/scripts/oc-tests/oc-common.sh: line 24: chectl: command not found
+ [[ 0 != \0 ]]
+ echo '[INFO] Job completed successfully.'
[INFO] Job completed successfully.
+ rm -rf /home/ndp/projects/ibuziuk/che-operator/tmp
+ exit 0

@tolusha
Copy link
Contributor

tolusha commented Nov 15, 2023

chectl: command not found
It requires chectl

@dmytro-ndp
Copy link
Contributor

@tolusha: something went wrong on my local machine, and I hadn't managed building che-operator image.
I would be grateful if you could build the image from this PR and share it with me.

@tolusha
Copy link
Contributor

tolusha commented Nov 15, 2023

Please use the following image to test quay.io/abazko/operator:CRW-4592

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 15, 2023

something went wrong on my local machine, and I hadn't managed building che-operator image.

@dmytro-ndp why are you using local machine and not the dogfooding instance? make docker-build docker-push IMG=<img> should work I belive

@dmytro-ndp
Copy link
Contributor

@ibuziuk :

@dmytro-ndp why are you using local machine and not the dogfooding instance?

Because I didn't know that it was possible to build che-operator image on Che dogfooding instance
Thanks to your hint I have successfully built quay.io/dmytro_ndp/che-operator:pr-1777 on Che dogfooding.

I have a question about PR testing.
PR description said that we should apply next che cluster patch when install Dev Spaces:

spec:
  components:
    cheServer:
      extraProperties:
        JAVA_OPTS : '-XX:MaxRAMPercentage=85.0'

Is it really needed taking into account that there is already DefaultJavaOpts = "-XX:MaxRAMPercentage=85.0" in pkg/common/constants/constants.go ?

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 15, 2023

@dmytro-ndp no worries, my point was that we should use Eclipse Che for development / verification as much as possible ;-)

PR description said that we should apply next che cluster patch when install Dev Spaces

PR description is saying about testing Dev Spaces 3.9.1 and 3.8.0. For upstream Eclipse Che verification patching CR is not needed, but it is important to note that the issue was reproducible only on Dev Spaces, not Eclipse Che. That's great to verify upstream functionality, but before merging this PR please also do the verification of Dev Spaces according to the PR description.

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 15, 2023

@ibuziuk : I have successfully tested Dev Spaces 3.9.1 deployed using quay.io/dmytro_ndp/che-operator:pr-1777 built from this PR to OCP 4.14 cluster with FIPS enabled (Cluster Bot command launch 4.13 azure,fips):

  • Check that the FIPS mode is enabled on the cluster

if was enabled

Screenshot

Screenshot from 2023-11-15 21-01-41

  • Check the value of the JAVA_OPTS env var in che-server / devspaces pod as well as in the che configmap - it should be -XX:MaxRAMPercentage=85.0
Screenshot

Screenshot from 2023-11-15 20-52-21

  • start / stop a few workspace and verify that workspace startup has no issues / no errors in the che-server logs

Quarkus and Lombok workspaces has started successfully.
There were no error messages when start workspace in devspaces pod logs:

Screenshot

Screenshot from 2023-11-15 20-59-50

TODO: testing DS 3.8.0 / Eclipse Che next.

@dmytro-ndp
Copy link
Contributor

@ibuziuk : about

On Dev Spaces 3.8 if this flag is removed, che-server should fail to start

DS 3.8.0 server DID NOT fail to start on OCP 4.14 azure with fips, and JAVA_OPTS=-XX:MaxRAMPercentage=85.0:

oc get cm cluster-config-v1 -n kube-system -o json | jq -r '.data' | grep -i "fips" command

Screenshot from 2023-11-16 15-03-02

JAVA_OPTS env var in devspaces pod

Screenshot from 2023-11-16 14-59-53

devspaces pod logs

Screenshot from 2023-11-16 15-02-16

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 16, 2023

@dmytro-ndp thanks for the review, this does not look expected. Would it be possible to try the same flow against 3.7?

Basically, I tried it in mid-July and was able to reproduce this error - https://issues.redhat.com/browse/CRW-4592?focusedId=22646733&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-22646733

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 16, 2023

@ibuziuk: DS 3.7.2 server had also successfully started on OCP 4.14 azure with fips, and JAVA_OPTS=-XX:MaxRAMPercentage=85.0, and I had successfully created workspace in there.

Here is server logs: https://gist.github.com/dmytro-ndp/1c0d536000e27e6b893f9c52552b502f

Do you think it could be related to OCP version = 4.14?

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

PR looks good to merge, because Eclipse Che Next has been successfully deployed to OCP 4.13 with FIPS enabled using "quay.io/dmytro_ndp/che-operator:pr-1777" image built from this PR (https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Che/job/ocp/job/basic/job/install-che-to-ocp/89/) as well as Dev Spaces 3.9.1 to OCP 4.14 with FIPS enabled.

Copy link

openshift-ci bot commented Nov 17, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dmytro-ndp, ibuziuk, tolusha

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

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

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 21, 2023

@dmytro-ndp Thanks for the review, I was able to reproduce the error back in the summer - https://issues.redhat.com/secure/attachment/13005118/13005118_devspaces-56c78b84-ddb6d-devspaces.log

Eclipse Che Api Core: Build info '7.67.1.redhat-00004' scmRevision 'ed152183c9f474dd6ea93daa5883b9a0901bd257'

Which Dev Spaces version does it correspond to?

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 21, 2023

@ibuziuk : I have tested DS 3.7.2 with Eclipse Che Api Core: Build info '7.67.1.redhat-00009' scmRevision '8cb9328e36c05d7aa07ece1f23d5f3ebc2f1f7fc' implementationVersion '7.67.1.redhat-00009'

You can find devspaces logs in my message above: #1777 (comment)

Which Dev Spaces version does it correspond to?

It could be DS 3.7.0 or 3.7.1

@ibuziuk
Copy link
Member Author

ibuziuk commented Nov 21, 2023

ack, thanks
going to re-test against 3.7.0 - the issue should be reproducible against this version

@devstudio-release
Copy link

Build 3.11 :: copyIIBsToQuay/2217: SUCCESS

3.11
arches = x86_64, s390x, ppc64le;
  * LATEST DS OPERATOR BUNDLE = <a href=https://quay.io/repository/devspaces/devspaces-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devspaces-operator-bundle:3.11-109
  * LATEST DWO OPERATOR BUNDLE = <a href=https://quay.io/repository/devworkspace/devworkspace-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devworkspace-operator-bundle:0.21-7
+ s390x-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630133-533291-s390x
  + quay.io/devspaces/iib:3.11-v4.14-s390x
  + quay.io/devspaces/iib:next-v4.14-s390x
  + quay.io/devspaces/iib:3.11-v4.13-630125-533286-s390x
  + quay.io/devspaces/iib:3.11-v4.13-s390x
  + quay.io/devspaces/iib:next-v4.13-s390x
  + quay.io/devspaces/iib:3.11-v4.12-630242-533281-s390x
  + quay.io/devspaces/iib:3.11-v4.12-s390x
  + quay.io/devspaces/iib:next-v4.12-s390x
  + quay.io/devspaces/iib:3.11-v4.11-630239-533277-s390x
  + quay.io/devspaces/iib:3.11-v4.11-s390x
  + quay.io/devspaces/iib:next-v4.11-s390x
+ x86_64-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630133-533291-x86_64
  + quay.io/devspaces/iib:3.11-v4.14-x86_64
  + quay.io/devspaces/iib:next-v4.14-x86_64
  + quay.io/devspaces/iib:3.11-v4.13-630125-533286-x86_64
  + quay.io/devspaces/iib:3.11-v4.13-x86_64
  + quay.io/devspaces/iib:next-v4.13-x86_64
  + quay.io/devspaces/iib:3.11-v4.12-630242-533281-x86_64
  + quay.io/devspaces/iib:3.11-v4.12-x86_64
  + quay.io/devspaces/iib:next-v4.12-x86_64
  + quay.io/devspaces/iib:3.11-v4.11-630239-533277-x86_64
  + quay.io/devspaces/iib:3.11-v4.11-x86_64
  + quay.io/devspaces/iib:next-v4.11-x86_64
+ ppc64le-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630133-533291-ppc64le
  + quay.io/devspaces/iib:3.11-v4.14-ppc64le
  + quay.io/devspaces/iib:next-v4.14-ppc64le
  + quay.io/devspaces/iib:3.11-v4.13-630125-533286-ppc64le
  + quay.io/devspaces/iib:3.11-v4.13-ppc64le
  + quay.io/devspaces/iib:next-v4.13-ppc64le
  + quay.io/devspaces/iib:3.11-v4.12-630242-533281-ppc64le
  + quay.io/devspaces/iib:3.11-v4.12-ppc64le
  + quay.io/devspaces/iib:next-v4.12-ppc64le
  + quay.io/devspaces/iib:3.11-v4.11-630239-533277-ppc64le
  + quay.io/devspaces/iib:3.11-v4.11-ppc64le
  + quay.io/devspaces/iib:next-v4.11-ppc64le

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.11 :: sync-to-downstream_3.x/5414: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/5287 triggered; /job/DS_CI/job/dsc_3.x triggered;

@devstudio-release
Copy link

Build 3.11 :: operator-bundle_3.x/2335: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/5414 triggered

@devstudio-release
Copy link

Build 3.11 :: copyIIBsToQuay/2218: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1601: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1601: SUCCESS

3.11.0-CI

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.11 :: copyIIBsToQuay/2220: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.11 :: sync-to-downstream_3.x/5416: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/5289 triggered; /job/DS_CI/job/dsc_3.x triggered;

@devstudio-release
Copy link

Build 3.11 :: operator-bundle_3.x/2336: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/5416 triggered

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1602: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1602: SUCCESS

3.11.0-CI

@devstudio-release
Copy link

Build 3.11 :: copyIIBsToQuay/2220: SUCCESS

3.11
arches = x86_64, s390x, ppc64le;
  * LATEST DS OPERATOR BUNDLE = <a href=https://quay.io/repository/devspaces/devspaces-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devspaces-operator-bundle:3.11-111
  * LATEST DWO OPERATOR BUNDLE = <a href=https://quay.io/repository/devworkspace/devworkspace-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devworkspace-operator-bundle:0.21-7
+ s390x-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630343-533291-s390x
  + quay.io/devspaces/iib:3.11-v4.14-s390x
  + quay.io/devspaces/iib:next-v4.14-s390x
  + quay.io/devspaces/iib:3.11-v4.13-630340-533286-s390x
  + quay.io/devspaces/iib:3.11-v4.12-630441-533281-s390x
  + quay.io/devspaces/iib:3.11-v4.12-s390x
  + quay.io/devspaces/iib:next-v4.12-s390x
  + quay.io/devspaces/iib:3.11-v4.11-630436-533277-s390x
  + quay.io/devspaces/iib:3.11-v4.11-s390x
  + quay.io/devspaces/iib:next-v4.11-s390x
+ x86_64-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630343-533291-x86_64
  + quay.io/devspaces/iib:3.11-v4.13-630340-533286-x86_64
  + quay.io/devspaces/iib:3.11-v4.12-630441-533281-x86_64
  + quay.io/devspaces/iib:3.11-v4.12-x86_64
  + quay.io/devspaces/iib:next-v4.12-x86_64
  + quay.io/devspaces/iib:3.11-v4.11-630436-533277-x86_64
  + quay.io/devspaces/iib:3.11-v4.11-x86_64
  + quay.io/devspaces/iib:next-v4.11-x86_64
+ ppc64le-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.11-v4.14-630343-533291-ppc64le
  + quay.io/devspaces/iib:3.11-v4.14-ppc64le
  + quay.io/devspaces/iib:next-v4.14-ppc64le
  + quay.io/devspaces/iib:3.11-v4.13-630340-533286-ppc64le
  + quay.io/devspaces/iib:3.11-v4.12-630441-533281-ppc64le
  + quay.io/devspaces/iib:3.11-v4.12-ppc64le
  + quay.io/devspaces/iib:next-v4.12-ppc64le
  + quay.io/devspaces/iib:3.11-v4.11-630436-533277-ppc64le
  + quay.io/devspaces/iib:3.11-v4.11-ppc64le
  + quay.io/devspaces/iib:next-v4.11-ppc64le

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.11 :: get-sources-rhpkg-container-build_3.x/5310: SUCCESS

devspaces-operator-bundle : 3.x :: Build 57278576 : operator-bundle-3.11-115
SUCCESS; copied to quay

@devstudio-release
Copy link

Build 3.11 :: sync-to-downstream_3.x/5436: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/5310 triggered; /job/DS_CI/job/dsc_3.x triggered;

@devstudio-release
Copy link

Build 3.11 :: operator-bundle_3.x/2340: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/5436 triggered

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1606: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.11 :: dsc_3.x/1606: SUCCESS

3.11.0-CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants