devfile / devworkspace-operator

Apache License 2.0
67 stars 55 forks source link

Apply ssh askpass flow for the workspace container #1307

Closed vinokurig closed 2 months ago

vinokurig commented 3 months ago

What does this PR do?

What issues does this PR fix or reference?

fixes https://github.com/devfile/devworkspace-operator/issues/1295

Is it tested? How?

  1. Start a workspace and check that the DISPLAY and SSH_ASKPASS environment variables are set
  2. Check that the /.ssh-askpass/ssh-askpass.sh file is present and have the right content.

    PR Checklist

openshift-ci[bot] commented 3 months ago

Hi @vinokurig. Thanks for your PR.

I'm waiting for a devfile member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 months ago

/test v8-devworkspace-operator-e2e, v8-che-happy-path

openshift-ci[bot] commented 3 months ago

@vinokurig: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to [this](https://github.com/devfile/devworkspace-operator/pull/1307#issuecomment-2290726961): >/test v8-devworkspace-operator-e2e, v8-che-happy-path 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.
AObuchow commented 3 months ago

@vinokurig Also, please run make fmt as the format validation CI check is currently failing

vinokurig commented 3 months ago

@AObuchow Thank you for the review, I've added another commit to add a post start event to initialise ssh-agent and add the workspace ssh-key. Please take a look.

vinokurig commented 2 months ago

@AObuchow

it seems we're still referencing /home/user/ in two lines of the postStart event: here and here. Could you please change these to $HOME or ~?

fixed

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 2.77778% with 70 lines in your changes missing coverage. Please review.

Project coverage is 52.63%. Comparing base (adc8b5a) to head (405ec53). Report is 64 commits behind head on main.

Files with missing lines Patch % Lines
pkg/provision/workspace/ssh.go 0.00% 50 Missing :warning:
pkg/library/env/workspaceenv.go 0.00% 14 Missing :warning:
controllers/workspace/devworkspace_controller.go 25.00% 4 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1307 +/- ## ========================================== + Coverage 52.52% 52.63% +0.10% ========================================== Files 84 85 +1 Lines 7642 7976 +334 ========================================== + Hits 4014 4198 +184 - Misses 3335 3477 +142 - Partials 293 301 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AObuchow commented 2 months ago

/ok-to-test

dkwon17 commented 2 months ago

LGTMl, I just have one comment

vinokurig commented 2 months ago

@AObuchow

LGTM, awesome work @vinokurig 😁 Please squash all your commits into a single commit (I think you could squash all your commits into your first commit)

You could also add "Fixes https://github.com/devfile/devworkspace-operator/issues/1295" to the commit description.

thanks, done

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AObuchow, dkwon17, 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