Introduce Git step command #751

HeavyWombat · 2021-04-29T12:27:59Z

Changes

In order to replace Tekton Git resource, a replacement container image
is required that performs a Git clone.

Add Git step command which wraps git CLI to clone user repositories
with different authentication options.

Submitter Checklist

Includes tests if functionality changed/was added
Includes docs if changes are user-facing
Set a kind label on this PR
Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Added Git command as a replacement for the Tekton Git Resource.

SaschaSchwarze0

Which make target can be used to build the image?

SaschaSchwarze0 · 2021-04-29T12:56:46Z

cmd/git/main.go

+	isSSHGitURL := sshGitURLRegEx.MatchString(flagValues.url)
+	switch {
+	case hasPrivateKey && !isSSHGitURL:
+		return typeUndef, &ExitError{Code: 110, Message: "Credential/URL inconsistency: SSH credentials provided, but URL is not a SSH Git URL"}


This is different to the current behavior where there is "only" a warning. Our current e2e tests - when running with private repo support - also adds the credential to all Builds ignoring if it is to test a private repo or not if I remember correctly. On the other hand, it is not a bad idea to do this.

Maybe we can have a call about this. I took the liberty to change the behavior a bit and that we then can discuss the pros and cons.

SaschaSchwarze0 · 2021-04-29T12:59:12Z

Related to #696

HeavyWombat · 2021-04-29T13:07:11Z

Which make target can be used to build the image?

Is coming up next. I wanted to code to be out already for some feedback.

adambkaplan · 2021-04-29T14:50:44Z

There's a huge amount of overlap here with what we've done in OpenShift builds. We have a lot of battle-tested code to manage the cloning of git source. It is currently spread across several repos - I can do a deep dive to see where these bits are and if they can be repackaged for Shipwright.

HeavyWombat · 2021-04-30T09:52:47Z

Which make target can be used to build the image?

Is coming up next. I wanted to code to be out already for some feedback.

We eventually decided against a Makefile target as ideally it will be magically done by the build through a ko reference.

qu1queee

From my quick overview today, it looks good 👍🏾. I would like to go over the credentials part tomorrow. Leaving you some open q´s from my side for now.

cmd/git/README.md

cmd/git/main.go

qu1queee

I tested the different auth methods locally. All of the seems to be running smoothly, pretty nice behaviour, thanks for this!

In order to replace Tekton Git resource, a replacement container image is required that performs a Git clone. Add Git step command which wraps `git` CLI to clone user repositories with different authentication options.

Since the key provided via a secret can have undesirable file permissions, it will end up failing due to SSH sanity checks. Create temporary SSH private key using `0400` file permissions.

qu1queee

/lgtm

SaschaSchwarze0

/approve

openshift-ci-robot · 2021-05-06T08:36:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

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~~ [SaschaSchwarze0]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot requested review from mattcui and qu1queee April 29, 2021 12:28

openshift-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Apr 29, 2021

HeavyWombat requested review from SaschaSchwarze0 and removed request for mattcui April 29, 2021 12:28

openshift-ci-robot added release-note Label for when a PR has specified a release note and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 29, 2021

HeavyWombat added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 29, 2021

SaschaSchwarze0 reviewed Apr 29, 2021

View reviewed changes

HeavyWombat marked this pull request as draft April 29, 2021 14:07

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 29, 2021

qu1queee requested a review from adambkaplan April 30, 2021 07:28

HeavyWombat marked this pull request as ready for review April 30, 2021 07:30

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2021

HeavyWombat requested a review from SaschaSchwarze0 April 30, 2021 07:30

qu1queee requested changes May 3, 2021

View reviewed changes

cmd/git/README.md Show resolved Hide resolved

cmd/git/main.go Show resolved Hide resolved

cmd/git/main.go Show resolved Hide resolved

qu1queee reviewed May 4, 2021

View reviewed changes

cmd/git/main.go Show resolved Hide resolved

qu1queee requested changes May 4, 2021

View reviewed changes

HeavyWombat added 2 commits May 6, 2021 09:41

Introduce Git step command

09cc23d

In order to replace Tekton Git resource, a replacement container image is required that performs a Git clone. Add Git step command which wraps `git` CLI to clone user repositories with different authentication options.

Create temporary SSH private key

aa28765

Since the key provided via a secret can have undesirable file permissions, it will end up failing due to SSH sanity checks. Create temporary SSH private key using `0400` file permissions.

qu1queee self-requested a review May 6, 2021 08:35

qu1queee approved these changes May 6, 2021

View reviewed changes

openshift-ci-robot assigned qu1queee May 6, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 6, 2021

SaschaSchwarze0 reviewed May 6, 2021

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2021

SaschaSchwarze0 approved these changes May 6, 2021

View reviewed changes

openshift-merge-robot merged commit 67f6df4 into shipwright-io:master May 6, 2021

HeavyWombat deleted the add/git-step-command branch May 17, 2021 16:16

adambkaplan added this to the release-v0.5.0 milestone Jun 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Git step command #751

Introduce Git step command #751

HeavyWombat commented Apr 29, 2021 •

edited

Loading

SaschaSchwarze0 left a comment

SaschaSchwarze0 Apr 29, 2021 •

edited

Loading

HeavyWombat Apr 29, 2021

SaschaSchwarze0 commented Apr 29, 2021

HeavyWombat commented Apr 29, 2021

adambkaplan commented Apr 29, 2021

HeavyWombat commented Apr 30, 2021

qu1queee left a comment

qu1queee left a comment

qu1queee left a comment

SaschaSchwarze0 left a comment

openshift-ci-robot commented May 6, 2021

Introduce Git step command #751

Introduce Git step command #751

Conversation

HeavyWombat commented Apr 29, 2021 • edited Loading

Changes

Submitter Checklist

Release Notes

SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

SaschaSchwarze0 Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

HeavyWombat Apr 29, 2021

Choose a reason for hiding this comment

SaschaSchwarze0 commented Apr 29, 2021

HeavyWombat commented Apr 29, 2021

adambkaplan commented Apr 29, 2021

HeavyWombat commented Apr 30, 2021

qu1queee left a comment

Choose a reason for hiding this comment

qu1queee left a comment

Choose a reason for hiding this comment

qu1queee left a comment

Choose a reason for hiding this comment

SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented May 6, 2021

HeavyWombat commented Apr 29, 2021 •

edited

Loading

SaschaSchwarze0 Apr 29, 2021 •

edited

Loading