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

Patch Rancher shell image to v0.1.26 #813

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

w13915984028
Copy link
Member

@w13915984028 w13915984028 commented Aug 14, 2024

Problem:

CVE

Solution:

Patch rancher initial shell image

Related Issue:

harvester/harvester#6283

Test plan:

  1. A newly installed cluster:

cluster is ready
image

  1. Expected values are correct
$ cat /usr/share/rancher/rancherd/config.yaml.d/50-defaults.yaml
rancherValues:
  rancherImagePullPolicy: IfNotPresent
  rancherImage: rancher/rancher
  rancherImageTag: v2.8.5
  noDefaultAdmin: false
  features: multi-cluster-management=false,multi-cluster-management-agent=false
  useBundledSystemChart: true
  bootstrapPassword: admin
  hostPort: 0
  global:
    cattle:
      psp:
        enabled: false
  extraEnv:
    - name: CATTLE_SHELL_IMAGE
      value: "rancher/shell:v0.1.26"
rancherInstallerImage: rancher/system-agent-installer-rancher:v2.8.5

$ crictl image ls | grep shell
docker.io/rancher/shell                                                     v0.1.26                                     33dc949c57f81       304MB


$ cat /oem/harvester.config

...
runtimeversion: v1.28.12+rke2r1
rancherversion: v2.8.5
harvesterchartversion: 0.0.0-v1.3-ca72622e
monitoringchartversion: 103.0.3+up45.31.1
systemsettings:
    ntp-servers: '{"ntpServers":["0.suse.pool.ntp.org"]}'
loggingchartversion: 103.0.0+up3.17.10
$ kubectl get settings.management.cattle.io shell-image
NAME          VALUE
shell-image   rancher/shell:v0.1.26


harv21:/home/rancher # kubectl get settings.management.cattle.io shell-image -oyaml
apiVersion: management.cattle.io/v3
customized: false
default: rancher/shell:v0.1.24
kind: Setting
metadata:
  creationTimestamp: "2024-08-14T17:08:29Z"
  generation: 1
  name: shell-image
  resourceVersion: "1370"
  uid: e73fba98-d3f4-48b2-bf25-ed531ee69f46
source: env
value: rancher/shell:v0.1.26

@w13915984028 w13915984028 requested a review from bk201 August 14, 2024 18:03
@w13915984028
Copy link
Member Author

cc @mallardduck, it works, thanks.

@bk201
Copy link
Member

bk201 commented Aug 15, 2024

LGTM, ok to provision cluster in air-gap env.

@bk201 bk201 merged commit 9d670c9 into harvester:v1.3 Aug 15, 2024
7 checks passed
@w13915984028
Copy link
Member Author

Note: Rancher v2.9 will user a newer shell version defaultShellVersion: rancher/shell:v0.2.1, this PR is NOT needed to forward to Harvester v1.4 branch.

https://github.com/rancher/rancher/blob/3f7c25803c00e5311d89d1144b3c6684b288ed51/pkg/settings/setting.go#L128C78-L128C97

https://github.com/rancher/rancher/blob/a9946d267616beb1be27c3142a4ebaf6186ba3e7/build.yaml#L4

@bk201
Copy link
Member

bk201 commented Aug 15, 2024

@w13915984028 say a user creates a new v1.3.2 cluster with this change. If he upgrades Harvester to v1.4.0 (supposed to use Rancher 2.9), is the shell image updated to the newer version in this case?

@w13915984028
Copy link
Member Author

@w13915984028 say a user creates a new v1.3.2 cluster with this change. If he upgrades Harvester to v1.4.0 (supposed to use Rancher 2.9), is the shell image updated to the newer version in this case?

@bk201 This is a good question, I planed to investigate together with:
harvester/harvester#6277

Both Rancher and Harvester, has the setting which was initialized from Source code, instead of chart element like configmap. And this setting has a default value.

https://github.com/harvester/harvester/blob/78193016883c6738a12125dd4c5349d18709bd86/pkg/settings/settings.go#L47

Rancher shell-image

harv31:/home/rancher # kubectl get settings.management.cattle.io shell-image -oyaml
apiVersion: management.cattle.io/v3
customized: false
default: rancher/shell:v0.1.23
kind: Setting
metadata:
  creationTimestamp: "2024-08-06T13:05:10Z"
  generation: 1
  name: shell-image
  resourceVersion: "1358"
  uid: 4752640a-6b51-46ac-a6e7-24568a9af084
source: ""
value: ""

That will bring potential issues like:

  1. When such a setting was created on say Harvester v1.3.1 with default value A; after upgraded to v1.4.0, the default value A may not be updated to the new default value B on Harvester v1.4.0. (need to confirm)

  2. When user set a value on v1.3.1, but after upgrade to v1.4.0, the default value is newer than the user set value, should we revert the user set value automatically?

LH has a configmap to init the settings from chart; and the controller checks the configmap to decide which value should be adopt, looks to be a bit better. WDYT?

also cc @starbops regarding the upgrade part PR.

@bk201
Copy link
Member

bk201 commented Aug 15, 2024

@w13915984028 Thanks for the reply. Can you do some tests by setting the setting first and upgrading Rancher to see if a user-configured value is still used? I don't feel the default value matters here, since the value is already configured by a user, the value in the "value" field should be used.

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

Successfully merging this pull request may close these issues.

3 participants