search

WASdev / websphere-liberty-operator

Deploy and manage containerized Liberty applications on Kubernetes. Documentation: https://ibm.biz/wlo-docs
Apache License 2.0
9 stars 4 forks source link

[linter] Assess the results from dynamic linter scan #49

Closed leochr closed 2 years ago

leochr commented 2 years ago

Review the results from dynamic linter scan.

leochr commented 2 years ago

@mcurran-us please attach the dynamic linter scan results. Thanks.

mcurran-us commented 2 years ago

The items marked with an * are new items detected by the dynamic linter. The rest of the items have already been identified by the static linter. dynamic_lint_results.txt

leochr commented 2 years ago

@mcurran-us What's the yaml used for the WLO Custom Resources (CRs) for this? There are certain items that the IBM Certification requires, since we manage customer's application, we enable some configurations by default, but there are options in the CR to configure them (i.e. resource limits/requests, probes, affinity, autoscaling, etc)

leochr commented 2 years ago

@idlewis fyi, some dev items are still in progress, like adding .spec.license.* and .status.versions.reconciled, adding NetworkPolicy

mcurran-us commented 2 years ago

@leochr It uses the yamls that are defined in the operators config/samples directory.

leochr commented 2 years ago

Here is a sample WebSphereLibertyApplication with some additional configuration that'll address some linter issues. @idlewis please deploy it on your cluster to verify that it works fine

Edit: April 11, 2022: config updates are listed in the comment below

Edit: April 20, 2022: update livenessProbe initial delay to 70 seconds to satisfy linter condition, add architectue affinity

apiVersion: liberty.websphere.ibm.com/v1
kind: WebSphereLibertyApplication
metadata:
  name: wlo-app
spec:
  license:
    accept: true
  applicationImage: registry.connect.redhat.com/ibm/open-liberty-samples:springPetClinic
  expose: true
  replicas: 3
  probes:
    liveness:
      httpGet:
        path: /
        port: 9443
        scheme: HTTPS
      initialDelaySeconds: 70
    readiness:
      httpGet:
        path: /
        port: 9443
        scheme: HTTPS
      initialDelaySeconds: 60
  resources:
    limits:
      cpu: '1'
      memory: 1Gi
    requests:
      cpu: 500m
      memory: 400Mi
  statefulSet: {}
  serviceability:
    size: 1Gi
  affinity:
    architecture:
      - amd64
leochr commented 2 years ago

Sample CRs for day-2 operations:

apiVersion: liberty.websphere.ibm.com/v1
kind: WebSphereLibertyDump
metadata:
  name: websphereliberty-dump-sample
spec:
  podName: wlo-app-0
  include:
  - thread
  - heap
apiVersion: liberty.websphere.ibm.com/v1
kind: WebSphereLibertyTrace
metadata:
  name: websphereliberty-trace-sample
spec:
  podName: wlo-app-1
  traceSpecification: "*=info:com.ibm.ws.webcontainer*=all"
  maxFileSize: 20
  maxFiles: 5

Note that dump and trace are enabled on different pods intentionally. They could be enabled on the same pod, but dump shouldn't be requested while the trace is on, because that generates too much information and could crash the application pod.

mcurran-us commented 2 years ago

@leochr Are these changes to the sample yamls going to be incorporated into the operator code base?

leochr commented 2 years ago

@mcurran-us In the operator, we try to keep configuration to a minimum, since customers would deploy their apps and we don't know what the appropriate values would be for their apps. Some of the configurations would be added to the sample CRs in the operator, but not everything (i.e. we wouldn't have the serviceability or statefulset enabled, since that'll require storage configured on the cluster).

idlewis commented 2 years ago

There is one error from the dynamic scan that I'm not sure about: *[ERROR] scanned-pod-chobqif-dyn-0-websphereliberty-app-sample-6f94698c58-ptkg5.yaml: keys for pod security context not defined: spec.securityContext.runAsNonRoot OR spec.containers[0].securityContext.runAsNonRoot, spec.containers[0].securityContext.privileged, spec.containers[0].securityContext.readOnlyRootFilesystem, spec.containers[0].securityContext.allowPrivilegeEscalation (PodSecurityContextDefined) --> see references https://github.ibm.com/IBMPrivateCloud/content-verification/blob/master/docs/rules/PodSecurityContextDefined.md @leochr @halim-lee Should this be fixed by any recent changes?

leochr commented 2 years ago

@idlewis runAsNonRoot, privileged and allowPrivilegeEscalation should be fixed by https://github.com/WASdev/websphere-liberty-operator/pull/61

readOnlyRootFilesystem is not currently set. @halim-lee will have an update to RCO and then to WLO to specify that. readOnlyRootFilesystem is a bit more restrictive than the other fields, so we shouldn't set it to true by default. It'll be set to false. If that doesn't satisfy linter, then we'll need to add an override.

mcurran-us commented 2 years ago

Static linter results as of 4/6. Level of operator code is today, commit 90e4764. Images are from 1.0.0-20220405-1800. They are now running bundle validation as part of the linter results. However, I cannot run that locally. Therefore, there may be more errors that appear when this runs as part of the travis build. The items marked with an *** are new items detected by the dynamic linter. The rest of the items have already been identified by the static linter. dynamic-lint.txt

leochr commented 2 years ago

Updated the CR above to use for linter:

fyi @mcurran-us @idlewis

mcurran-us commented 2 years ago

Dynamic linter results for websphere-liberty-operator commit [849c9e1], image tag 1.0.0-20220410-1600. The items marked with an *** are new items detected by the dynamic linter. The rest of the items have already been identified by the static linter. Both the static and dynamic linters are now running in the travis build for the CASE. dynamic-lint-results-4-12-22.txt

idlewis commented 2 years ago

Leo #82 adds probes into the sample as per your snippet above, I've confirmed that it does fix some (but not all) of the dynamic linter errors

mcurran-us commented 2 years ago

Dynamic linter results for websphere-liberty-operator commit [https://github.com/WASdev/websphere-liberty-operator/commit/30b9d6495f73afb76e9fa48c07cb7ca33858ddd0], image tag 1.0.0-20220418-1800 dynamic-lint-results-4-19-22.txt

leochr commented 2 years ago

@mcurran-us

False-positive (requires permanent linter override):

CV Test's CR need to be updated to resolve these:

The rest of the errors require changes to operator or sample app CR. We are investigating. In the mean time, they could be temporarily overridden. Thanks.

mcurran-us commented 2 years ago

@leochr The CV tests run on amd64.

leochr commented 2 years ago

@mcurran-us thanks, we can set spec.affinity.architecture in the CV test CR to satisfy the PodHasArchBasedNodeAffinity linter requirement. Updated the sample CR above to include the snippet.

leochr commented 2 years ago

@mcurran-us please share the latest dynamic linter scan results (from the GM candidate). Thanks

mcurran-us commented 2 years ago

@leochr You can see the latest dynamic linter results here: https://certification.cloudpaklab.ibm.com/#/certifications/certView/fc36702e-cca8-41c8-9987-8754a96717b0/lint/results

leochr commented 2 years ago

Thank you @mcurran-us

@BradleyMayo could you please take a look at the latest dynamic scan results to see if there are any new items (including warning/review)? Thank you

BradleyMayo commented 2 years ago

There are no errors currently and none of the items are new. They were all on the last scan