add transport package with server implementation #4

alaypatel07 · 2021-11-08T22:19:39Z

Describe what this PR does
This PR adds a transport interface along with the server implementation. This will be followed by accompanying client implementation.

Signed-off-by: Alay Patel <[email protected]>

transport/stunnel/server.go

JohnStrunk · 2021-11-10T17:53:49Z

transport/stunnel/server.go

+socket = l:TCP_NODELAY=1
+socket = r:TCP_NODELAY=1
+debug = 7
+sslVersion = TLSv1.2


Why are we specifying 1.2 instead of 1.3?

will update

JohnStrunk · 2021-11-10T17:54:43Z

transport/stunnel/server.go

+socket = r:TCP_NODELAY=1
+debug = 7
+sslVersion = TLSv1.2
+[rsync]


We should probably use a generic name here. The tunnel itself doesn't really have anything to do w/ rsync (even that's going to be its primary use).

transport/stunnel/server.go

transport/transport.go

JohnStrunk · 2021-11-10T20:35:30Z

transport/transport.go

+
+type Type string
+
+func GenerateSSLCert() (*bytes.Buffer, *bytes.Buffer, *bytes.Buffer, error) {


This function could use some docs or perhaps named return values. 3 different *bytes.Buffer is a bit unwieldy.

JohnStrunk · 2021-11-10T20:36:39Z

transport/transport.go

+	}
+
+	subj := pkix.Name{
+		CommonName:         "backube.dev",


Does this require a domain name or can we just leave it at "backube"?

JohnStrunk · 2021-11-10T20:43:44Z

transport/transport.go

+
+type Options struct {
+	Labels map[string]string
+	Owners []metav1.OwnerReference


I notice when this gets used, it is directly assigned to the object's field instead of going through something like https://github.com/kubernetes-sigs/controller-runtime/blob/5f8befe79888a4a8529cda7a82797bd57e188158/pkg/controller/controllerutil/controllerutil.go#L96 that validates the reference (i.e., owner scoping, same namespace, etc). Does this get validated elsewhere?

JohnStrunk · 2021-11-10T20:50:36Z

transport/transport.go

+	MarkForCleanup(ctx context.Context, c client.Client, key, value string) error
+}
+
+type Options struct {


If consumers are going to use this directly, the fields need docs

alaypatel07

Responded to most of the comments. There are a few outstanding ones will work on addressing them as soon as possible. Thanks for the in-depth review.

transport/stunnel/server.go

alaypatel07 · 2021-11-12T18:15:05Z

transport/stunnel/server.go

+key = /etc/stunnel/certs/tls.key
+cert = /etc/stunnel/certs/tls.crt


will try PSK and let you know

alaypatel07 · 2021-11-12T18:25:28Z

transport/stunnel/server.go

+}
+
+func (s *server) prefixedName(name string) string {
+	return s.namespacedName.Name + "-server-" + name


create an issue for tracking this across all the packages

transport/transport.go

alaypatel07 · 2021-11-12T21:57:05Z

transport/stunnel/stunnel.go

+	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"


As discussed in the call, this is required because we'll have a client type soon, we agreed to keep it as of now.

Note: I am not a big fan of this either, but we agreed we have bigger fishes to fry :)

alaypatel07 · 2021-11-12T22:16:27Z

transport/stunnel/server.go

+}
+
+func (s *server) prefixedName(name string) string {
+	return s.namespacedName.Name + "-server-" + name


alaypatel07 · 2021-11-12T22:19:21Z

transport/transport.go

+
+type Options struct {
+	Labels map[string]string
+	Owners []metav1.OwnerReference


Signed-off-by: Alay Patel <[email protected]>

Co-authored-by: John Strunk <[email protected]> Signed-off-by: Alay Patel <[email protected]>

Signed-off-by: Alay Patel <[email protected]>

…te MarkForCleanup Signed-off-by: Alay Patel <[email protected]>

JohnStrunk

/lgtm

JohnStrunk · 2021-11-19T19:16:48Z

transport/stunnel/stunnel.go

+	switch {
+	case k8serrors.IsNotFound(err):
+		return false, nil
+	case err != nil:
+		return false, err
+	}


I learned something today 🤓

openshift-ci · 2021-11-19T19:20:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alaypatel07, JohnStrunk

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~~ [JohnStrunk,alaypatel07]

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

fix godoc for endpoint package

6ebf697

Signed-off-by: Alay Patel <[email protected]>

openshift-ci bot added do-not-merge/work-in-progress approved size/XL labels Nov 8, 2021

alaypatel07 force-pushed the transport branch 2 times, most recently from 556708b to 372ec51 Compare November 8, 2021 23:13

alaypatel07 added 2 commits November 9, 2021 10:20

add transport interface and stunnel-server implementation

20dc9af

Signed-off-by: Alay Patel <[email protected]>

add unit tests for transport server

a1e99e1

Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from 372ec51 to a1e99e1 Compare November 9, 2021 15:21

alaypatel07 changed the title ~~[WIP]: add transport package~~ add transport package with server implementation Nov 9, 2021

openshift-ci bot removed the do-not-merge/work-in-progress label Nov 9, 2021

alaypatel07 force-pushed the transport branch from 6bce99f to 6d741f8 Compare November 9, 2021 19:21

This was referenced Nov 9, 2021

Implement stunnel client #5

Closed

Implement a way to bypass the transport and use the transfer mechanism (rsync, rclone, etc) #6

Open

alaypatel07 force-pushed the transport branch from 6d741f8 to 65e9900 Compare November 9, 2021 22:55

refactor the functions that can be shared by client and server

8505c12

Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from 65e9900 to 8505c12 Compare November 9, 2021 23:08

alaypatel07 mentioned this pull request Nov 9, 2021

add stunnel client implementation #7

Merged

JohnStrunk reviewed Nov 10, 2021

View reviewed changes

alaypatel07 commented Nov 12, 2021

View reviewed changes

alaypatel07 force-pushed the transport branch 2 times, most recently from a4275c4 to 2addf39 Compare November 13, 2021 00:55

alaypatel07 and others added 2 commits November 12, 2021 19:57

fixups: add comments and documentations

96e9b58

Signed-off-by: Alay Patel <[email protected]>

fixups: context, flip NamespacedName, type cast "stunnel"

402af80

Co-authored-by: John Strunk <[email protected]> Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from 2addf39 to 402af80 Compare November 13, 2021 00:57

openshift-ci bot added size/XXL and removed size/XL labels Nov 18, 2021

alaypatel07 force-pushed the transport branch from a03075b to 09dc1cd Compare November 18, 2021 03:49

add tls authentication for transport

e10d983

Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from 09dc1cd to e10d983 Compare November 18, 2021 03:57

alaypatel07 mentioned this pull request Nov 18, 2021

e2e test for stunnel #11

Open

update unit tests to check for valid stunnel secret

75c304a

Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from f9ee120 to 75c304a Compare November 18, 2021 05:02

alaypatel07 requested a review from JohnStrunk November 18, 2021 06:16

stunnel: flip namespace-name, separate func for creating bundle, upda…

4b9f475

…te MarkForCleanup Signed-off-by: Alay Patel <[email protected]>

alaypatel07 force-pushed the transport branch from 6f5f3b7 to 4b9f475 Compare November 18, 2021 17:42

JohnStrunk approved these changes Nov 19, 2021

View reviewed changes

openshift-ci bot assigned JohnStrunk Nov 19, 2021

openshift-ci bot added the lgtm label Nov 19, 2021

openshift-merge-robot merged commit 25abd17 into backube:main Nov 19, 2021

alaypatel07 mentioned this pull request Dec 1, 2021

Transfer package #12

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add transport package with server implementation #4

add transport package with server implementation #4

alaypatel07 commented Nov 8, 2021 •

edited

Loading

JohnStrunk Nov 10, 2021

alaypatel07 Nov 12, 2021

JohnStrunk Nov 10, 2021

JohnStrunk Nov 10, 2021

JohnStrunk Nov 10, 2021

JohnStrunk Nov 10, 2021

alaypatel07 Nov 12, 2021

JohnStrunk Nov 10, 2021

alaypatel07 left a comment

alaypatel07 Nov 12, 2021

alaypatel07 Nov 12, 2021

alaypatel07 Nov 12, 2021

alaypatel07 Nov 12, 2021

alaypatel07 Nov 12, 2021

JohnStrunk left a comment

JohnStrunk Nov 19, 2021

openshift-ci bot commented Nov 19, 2021


		type Type string

		func GenerateSSLCert() (bytes.Buffer, bytes.Buffer, *bytes.Buffer, error) {

		key = /etc/stunnel/certs/tls.key
		cert = /etc/stunnel/certs/tls.crt

add transport package with server implementation #4

add transport package with server implementation #4

Conversation

alaypatel07 commented Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alaypatel07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnStrunk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 19, 2021

alaypatel07 commented Nov 8, 2021 •

edited

Loading