devfile / devworkspace-operator

Apache License 2.0
67 stars 55 forks source link

add SSH agent postStart event only if SSH key has a passphrase & experimental features enabled #1341

Closed AObuchow closed 2 weeks ago

AObuchow commented 3 weeks ago

What does this PR do?

The SSH agent initialization postStart event is now only injected under the following conditions:

The intention of this PR is to ensure the SSH agent initialization postStart event is only injected if user's opt-in by configuring the DWOC accordingly, and provide a passphrase in their SSH key.

However, this is only a temporary workaround for DWO 0.31.2. After this PR, we should reconsider how this postStart event should be injected. I've mentioned 2 potential ideas in the long-term solution section of https://github.com/devfile/devworkspace-operator/issues/1340

What issues does this PR fix or reference?

1340

Is it tested? How?

First deploy DWO with the changes from this PR.

There are 4 scenarios to test:

  1. Starting a devworkspace with no SSH secret configured, and experimental features disabled in the DWOC. The SSH agent postStart event should not be injected, and this can be observed from the workspace pod's Kubernetes PostStart lifecycle hook.
  2. Starting a devworkspace with a passphrase-less SSH secret configured, and experimental features disabled in the DWOC. The SSH agent postStart event should not be injected.
  3. Starting a devworkspace with an SSH secret that contains a passphrase and experimental features disabled in the DWOC. The SSH agent postStart event should not be injected.
  4. Starting a devworkspace with an SSH secret that contains a passphrase and experimental features enabled in the DWOC. The SSH agent postStart event should be injected.

I recommend testing all 4 scenarios in order.

Scenario 1: no SSH secret configured; experimental features disabled

  1. Create a devworkspace:
cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: plain-devworkspace
spec:
  started: true
  routingClass: 'basic'
  template:
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
EOF
  1. Ensure the devworkspace starts up successfully.
  2. Verify that the devworkspace's pod does not have the SSH agent command in its PostStart lifecycle hook. The result of oc get pod <workspace-pod-name> -n $NAMESPACE -o json | jq '.spec.containers[0].lifecycle.postStart' should be null.
  3. You can now delete your devworkspace: oc delete dw plain-devworkspace -n $NAMESPACE

Scenario 2: SSH secret configured with no passphrase; experimental features disabled

  1. Configure an SSH key in the same namespace that you will create your devworkspace. Make sure you do not set the $PASSPHRASE environment variable when creating the SSH secret.
  2. Create a devworkspace.
  3. Ensure the devworkspace starts up successfully.
  4. Verify that the devworkspace's pod does not have the SSH agent command in its PostStart lifecycle hook.
  5. You can now delete your devworkspace: oc delete dw plain-devworkspace -n $NAMESPACE

Scenario 3: SSH secret configured with a passphrase; experimental features disabled

  1. Delete your SSH secret from Scenario 2:oc delete secret git-ssh-key -n $NAMESPACE
  2. Configure an SSH key in the same namespace that you will create your devworkspace. Make sure you set the $PASSPHRASE environment variable when creating the SSH secret.
  3. Create a devworkspace.
  4. Ensure the devworkspace starts up successfully.
  5. Verify that the devworkspace's pod does not have the SSH agent command in its PostStart lifecycle hook.
  6. You can now delete your devworkspace: oc delete dw plain-devworkspace -n $NAMESPACE

Scenario 4: SSH secret configured with a passphrase; experimental features enabled

  1. After testing Scenario 3, enable experimental features in the DWOC: oc edit dwoc -n $NAMESPACE
kind: DevWorkspaceOperatorConfig
apiVersion: controller.devfile.io/v1alpha1
config:
+  enableExperimentalFeatures: true
  routing:
    clusterHostSuffix: 192.168.49.2.nip.io
    defaultRoutingClass: basic
  workspace:
    imagePullPolicy: Always
  1. Create a devworkspace
  2. Ensure the devworkspace starts up successfully.
  3. Verify that the devworkspace's pod does have the SSH agent command in its PostStart lifecycle hook: oc get pod <workspace-pod-name> -n $NAMESPACE -o json | jq '.spec.containers[0].lifecycle.postStart'
{
  "exec": {
    "command": [
      "/bin/sh",
      "-c",
      "{\nSSH_ENV_PATH=$HOME/ssh-environment \\\n&& if [ -f /etc/ssh/passphrase ] && command -v ssh-add >/dev/null; \\\nthen ssh-agent | sed 's/^echo/#echo/' > $SSH_ENV_PATH \\\n&& chmod 600 $SSH_ENV_PATH && source $SSH_ENV_PATH \\\n&& ssh-add /etc/ssh/dwo_ssh_key < /etc/ssh/passphrase \\\n&& if [ -f $HOME/.bashrc ] && [ -w $HOME/.bashrc ]; then echo \"source ${SSH_ENV_PATH}\" >> $HOME/.bashrc; fi; fi\n} 1>/tmp/poststart-stdout.txt 2>/tmp/poststart-stderr.txt\n"
    ]
  }
}

PR Checklist

openshift-ci[bot] commented 3 weeks ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

openshift-ci[bot] commented 3 weeks ago

@vinokurig: changing LGTM is restricted to collaborators

In response to [this](https://github.com/devfile/devworkspace-operator/pull/1341#pullrequestreview-2418657214): > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
vinokurig commented 3 weeks ago

It would be nice to add a documentation note e.g.

*Note:*  Specifying a passphrase for an SSH key is an experimental feature and is controlled by the `DevWorkspaceOperatorConfig.EnableExperimentalFeatures` option.
AObuchow commented 3 weeks ago

It would be nice to add a documentation note e.g.

*Note:*  Specifying a passphrase for an SSH key is an experimental feature and is controlled by the `DevWorkspaceOperatorConfig.EnableExperimentalFeatures` option.

+1 will add an extra commit for this

AObuchow commented 3 weeks ago

@dkwon17 thank you for the review :) will squash my fixup commits tomorrow & have this merged

openshift-ci[bot] commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AObuchow, dkwon17, ibuziuk, vinokurig

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/devfile/devworkspace-operator/blob/main/OWNERS)~~ [AObuchow,dkwon17] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-ci[bot] commented 2 weeks ago

New changes are detected. LGTM label has been removed.