Adding --tls option to domain create command (#1419)

* Adding --tls option to domain create command

* Adding client test for builder

* Update CHANGELOG.adoc

Co-authored-by: David Simansky <dsimansk@redhat.com>

* Edited domain create usage message

* Update docs/cmd/kn_domain_create.md

Co-authored-by: Roland Huß <rhuss@redhat.com>

* Update pkg/serving/v1alpha1/client.go

Co-authored-by: Roland Huß <rhuss@redhat.com>

* Update docs and usage message

* Changing test domain name in e2e for tls test

* fix e2e domain create test case

Co-authored-by: David Simansky <dsimansk@redhat.com>
Co-authored-by: Roland Huß <rhuss@redhat.com>

CHANGELOG.adoc

@@ -30,6 +30,14 @@
 | Show server error messages without any taints
 | https://github.com/knative/client/pull/1406[#1406]
 
+| 🎁
+| Adding `--tls` option to domain create command
+| https://github.com/knative/client/pull/1419[#1419]
+
+| 🎁
+| Add an `client.knative.dev/updateTimestamp` annotation to trigger a new revision when required
+| https://github.com/knative/client/pull/1364[#1364]
+
 | 🎁
 | Adding `--class` flag to broker create command
 | https://github.com/knative/client/pull/1402[#1402]

docs/cmd/kn_domain_create.md

@@ -10,8 +10,8 @@ kn domain create NAME
 
 ```
 
-  # Create a domain mappings 'hello.example.com' for Knative service 'hello'
+  # Create a domain mappings 'hello.example.com' for Knative service 'hello' with TLS secret set
-  kn domain create hello.example.com --ref hello
+  kn domain create hello.example.com --ref hello --tls my-cert-secret
 ```
 
 ### Options
@@ -20,6 +20,7 @@ kn domain create NAME
   -h, --help               help for create
   -n, --namespace string   Specify the namespace to operate in.
       --ref string         Addressable target reference for Domain Mapping. You can specify a Knative service, a Knative route. Examples: '--ref' ksvc:hello' or simply '--ref hello' for a Knative service 'hello', '--ref' kroute:hello' for a Knative route 'hello'. '--ref ksvc:mysvc:mynamespace' for a Knative service 'mysvc' in another namespace 'mynamespace', If a prefix is not provided, it is considered as a Knative service in the current namespace. If referring to a Knative service in another namespace, 'ksvc:name:namespace' combination must be provided explicitly.
+      --tls string         Enable TLS and point to the secret that holds the server certificate.
 ```
 
 ### Options inherited from parent commands

pkg/kn/commands/domain/create.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"k8s.io/apimachinery/pkg/util/validation"
 	knerrors "knative.dev/client/pkg/errors"
 	"knative.dev/client/pkg/kn/commands"
 	clientv1alpha1 "knative.dev/client/pkg/serving/v1alpha1"
@@ -32,8 +33,8 @@ func NewDomainMappingCreateCommand(p *commands.KnParams) *cobra.Command {
 		Use:   "create NAME",
 		Short: "Create a domain mapping",
 		Example: `
-  # Create a domain mappings 'hello.example.com' for Knative service 'hello'
+  # Create a domain mappings 'hello.example.com' for Knative service 'hello' with TLS secret set
-  kn domain create hello.example.com --ref hello`,
+  kn domain create hello.example.com --ref hello --tls my-cert-secret`,
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			if len(args) != 1 {
 				return errors.New("'kn domain create' requires the domain name given as single argument")
@@ -53,9 +54,14 @@ func NewDomainMappingCreateCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
+			errs := validation.IsDNS1123Subdomain(refFlags.tls)
+			if refFlags.tls != "" && len(errs) != 0 {
+				return fmt.Errorf("invalid secret name %q: %s", refFlags.tls, errs[0])
+			}
 			builder := clientv1alpha1.NewDomainMappingBuilder(name).
 				Namespace(namespace).
-				Reference(*reference)
+				Reference(*reference).
+				TLS(refFlags.tls)
 
 			client, err := p.NewServingV1alpha1Client(namespace)
 			if err != nil {
@@ -70,6 +76,7 @@ func NewDomainMappingCreateCommand(p *commands.KnParams) *cobra.Command {
 			return nil
 		},
 	}
+	cmd.Flags().StringVar(&refFlags.tls, "tls", "", "Enable TLS and point to the secret that holds the server certificate.")
 	commands.AddNamespaceFlags(cmd.Flags(), false)
 	refFlags.Add(cmd)
 	cmd.MarkFlagRequired("ref")

pkg/kn/commands/domain/create_test.go

@@ -29,13 +29,21 @@ func TestDomainMappingCreate(t *testing.T) {
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient(client.Namespace(), createService("foo"))
 
 	servingRecorder := client.Recorder()
-	servingRecorder.CreateDomainMapping(createDomainMapping("foo.bar", createServiceRef("foo", "default")), nil)
+	servingRecorder.CreateDomainMapping(createDomainMapping("foo.bar", createServiceRef("foo", "default"), ""), nil)
 
 	out, err := executeDomainCommand(client, dynamicClient, "create", "foo.bar", "--ref", "foo")
 	assert.NilError(t, err, "Domain mapping should be created")
 	assert.Assert(t, util.ContainsAll(out, "Domain", "mapping", "foo.bar", "created", "namespace", "default"))
 
 	servingRecorder.Validate()
+
+	servingRecorder.CreateDomainMapping(createDomainMapping("foo.bar", createServiceRef("foo", "default"), "my-tls-secret"), nil)
+
+	out, err = executeDomainCommand(client, dynamicClient, "create", "foo.bar", "--ref", "foo", "--tls", "my-tls-secret")
+	assert.NilError(t, err, "Domain mapping should be created")
+	assert.Assert(t, util.ContainsAll(out, "Domain", "mapping", "foo.bar", "created", "namespace", "default"))
+
+	servingRecorder.Validate()
 }
 func TestDomainMappingCreateWithError(t *testing.T) {
 	client := v1alpha1.NewMockKnServiceClient(t)
@@ -56,5 +64,9 @@ func TestDomainMappingCreateWithError(t *testing.T) {
 	assert.ErrorContains(t, err, "not found")
 	assert.Assert(t, util.ContainsAll(err.Error(), "services", "\"bar\"", "not", "found"))
 
+	_, err = executeDomainCommand(client, dynamicClient, "create", "foo.bar", "--ref", "foo", "--tls", "my-TLS-secret")
+	assert.ErrorContains(t, err, "invalid")
+	assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "name", "RFC 1123 subdomain"))
+
 	servingRecorder.Validate()
 }

pkg/kn/commands/domain/describe_test.go

@@ -95,7 +95,7 @@ func TestDomainMappingDescribeYAML(t *testing.T) {
 }
 
 func getDomainMapping() *servingv1alpha1.DomainMapping {
-	dm := createDomainMapping("foo.bar", createServiceRef("foo", "default"))
+	dm := createDomainMapping("foo.bar", createServiceRef("foo", "default"), "")
 	dm.TypeMeta = v1.TypeMeta{
 		Kind:       "DomainMapping",
 		APIVersion: "serving.knative.dev/v1alpha1",

pkg/kn/commands/domain/domain.go

@@ -44,6 +44,7 @@ func NewDomainCommand(p *commands.KnParams) *cobra.Command {
 
 type RefFlags struct {
 	reference string
+	tls       string
 }
 
 var refMappings = map[string]schema.GroupVersionResource{

pkg/kn/commands/domain/domain_test.go

@@ -130,7 +130,7 @@ func TestResolve(t *testing.T) {
 	}
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", myksvc, mykroute, myksvcInOther, mykrouteInOther)
 	for _, c := range cases {
-		i := &RefFlags{c.ref}
+		i := &RefFlags{reference: c.ref}
 		result, err := i.Resolve(context.Background(), dynamicClient, "default")
 		if c.destination != nil {
 			assert.DeepEqual(t, result, c.destination)
@@ -178,8 +178,8 @@ func createService(name string) *servingv1.Service {
 	}
 }
 
-func createDomainMapping(name string, ref duckv1.KReference) *servingv1alpha1.DomainMapping {
+func createDomainMapping(name string, ref duckv1.KReference, tls string) *servingv1alpha1.DomainMapping {
-	return clientservingv1alpha1.NewDomainMappingBuilder(name).Namespace("default").Reference(ref).Build()
+	return clientservingv1alpha1.NewDomainMappingBuilder(name).Namespace("default").Reference(ref).TLS(tls).Build()
 }
 
 func createServiceRef(service, namespace string) duckv1.KReference {

pkg/kn/commands/domain/list_test.go

@@ -29,8 +29,8 @@ import (
 func TestDomainMappingList(t *testing.T) {
 	client := v1alpha1.NewMockKnServiceClient(t)
 
-	dm1 := createDomainMapping("foo1", createServiceRef("foo1", "default"))
+	dm1 := createDomainMapping("foo1", createServiceRef("foo1", "default"), "")
-	dm2 := createDomainMapping("foo2", createServiceRef("foo2", "default"))
+	dm2 := createDomainMapping("foo2", createServiceRef("foo2", "default"), "")
 	servingRecorder := client.Recorder()
 	servingRecorder.ListDomainMappings(&servingv1alpha1.DomainMappingList{Items: []servingv1alpha1.DomainMapping{*dm1, *dm2}}, nil)
 

pkg/kn/commands/domain/update_test.go

@@ -33,8 +33,8 @@ func TestDomainMappingUpdate(t *testing.T) {
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient(client.Namespace(), createService("foo"), createService("bar"))
 
 	servingRecorder := client.Recorder()
-	servingRecorder.GetDomainMapping("foo.bar", createDomainMapping("foo.bar", createServiceRef("foo", "default")), nil)
+	servingRecorder.GetDomainMapping("foo.bar", createDomainMapping("foo.bar", createServiceRef("foo", "default"), ""), nil)
-	servingRecorder.UpdateDomainMapping(createDomainMapping("foo.bar", createServiceRef("bar", "default")), nil)
+	servingRecorder.UpdateDomainMapping(createDomainMapping("foo.bar", createServiceRef("bar", "default"), ""), nil)
 
 	out, err := executeDomainCommand(client, dynamicClient, "update", "foo.bar", "--ref", "bar")
 	assert.NilError(t, err, "Domain mapping should be updated")
@@ -59,7 +59,7 @@ func TestDomainMappingUpdateNotFound(t *testing.T) {
 func TestDomainMappingUpdateDeletingError(t *testing.T) {
 	client := v1alpha1.NewMockKnServiceClient(t)
 
-	deletingDM := createDomainMapping("foo.bar", createServiceRef("foo", "default"))
+	deletingDM := createDomainMapping("foo.bar", createServiceRef("foo", "default"), "")
 	deletingDM.DeletionTimestamp = &v1.Time{Time: time.Now()}
 
 	servingRecorder := client.Recorder()

pkg/serving/v1alpha1/client.go

@@ -160,6 +160,15 @@ func (b *DomainMappingBuilder) Reference(reference duckv1.KReference) *DomainMap
 	return b
 }
 
+// TLS for domainMapping builder
+func (b *DomainMappingBuilder) TLS(cert string) *DomainMappingBuilder {
+	if cert == "" {
+		return b
+	}
+	b.domainMapping.Spec.TLS = &servingv1alpha1.SecretTLS{SecretName: cert}
+	return b
+}
+
 // Build to return an instance of domainMapping object
 func (b *DomainMappingBuilder) Build() *servingv1alpha1.DomainMapping {
 	return b.domainMapping

pkg/serving/v1alpha1/client_test.go

@@ -82,6 +82,7 @@ func TestCreateDomainMapping(t *testing.T) {
 	serving, client := setup()
 	serviceName := "foo"
 	domainName := "foo.bar"
+	secretName := "tls-secret"
 	domainMapping := createDomainMapping(domainName, createServiceRef(serviceName, testNamespace))
 	serving.AddReactor("create", domainMappingResource,
 		func(a clienttesting.Action) (bool, runtime.Object, error) {
@@ -101,6 +102,16 @@ func TestCreateDomainMapping(t *testing.T) {
 		validateGroupVersionKind(t, domainMapping)
 	})
 
+	t.Run("create domain mapping with tls secret without error", func(t *testing.T) {
+		err := client.CreateDomainMapping(context.Background(), createDomainMappingWithTls(domainName, createServiceRef(serviceName, testNamespace), secretName))
+		assert.NilError(t, err)
+	})
+
+	t.Run("create domain mapping without tls secret without error", func(t *testing.T) {
+		err := client.CreateDomainMapping(context.Background(), createDomainMappingWithTls(domainName, createServiceRef(serviceName, testNamespace), ""))
+		assert.NilError(t, err)
+	})
+
 	t.Run("create  domain mapping with an error returns an error object", func(t *testing.T) {
 		err := client.CreateDomainMapping(context.Background(), createDomainMapping("unknown", createServiceRef(serviceName, testNamespace)))
 		assert.ErrorContains(t, err, "unknown")
@@ -203,6 +214,9 @@ func createDomainMapping(name string, ref duckv1.KReference) *servingv1alpha1.Do
 	return NewDomainMappingBuilder(name).Namespace("default").Reference(ref).Build()
 }
 
+func createDomainMappingWithTls(name string, ref duckv1.KReference, tls string) *servingv1alpha1.DomainMapping {
+	return NewDomainMappingBuilder(name).Namespace("default").Reference(ref).TLS(tls).Build()
+}
 func createServiceRef(service, namespace string) duckv1.KReference {
 	return duckv1.KReference{Name: service,
 		Kind:       "Service",

test/e2e/domain_mapping_test.go

@@ -19,6 +19,7 @@ package e2e
 
 import (
 	"testing"
+	"time"
 
 	"knative.dev/client/pkg/util"
 
@@ -52,10 +53,15 @@ func TestDomain(t *testing.T) {
 	domainUpdate(r, domainName, "foo")
 
 	t.Log("describe domain mappings")
-	domainDescribe(r, domainName)
+	domainDescribe(r, domainName, false)
 
 	t.Log("delete domain")
 	domainDelete(r, domainName)
+
+	t.Log("create domain with TLS")
+	domainCreateWithTls(r, "newdomain.com", "hello", "tls-secret")
+	time.Sleep(time.Second)
+	domainDescribe(r, "newdomain.com", true)
 }
 
 func domainCreate(r *test.KnRunResultCollector, domainName, serviceName string, options ...string) {
@@ -66,6 +72,14 @@ func domainCreate(r *test.KnRunResultCollector, domainName, serviceName string,
 	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "domain", "mapping", serviceName, domainName, "created", "namespace", r.KnTest().Kn().Namespace()))
 }
 
+func domainCreateWithTls(r *test.KnRunResultCollector, domainName, serviceName, tls string, options ...string) {
+	command := []string{"domain", "create", domainName, "--ref", serviceName, "--tls", tls}
+	command = append(command, options...)
+	out := r.KnTest().Kn().Run(command...)
+	r.AssertNoError(out)
+	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "domain", "mapping", domainName, "created", "namespace", r.KnTest().Kn().Namespace()))
+}
+
 func domainUpdate(r *test.KnRunResultCollector, domainName, serviceName string, options ...string) {
 	command := []string{"domain", "update", domainName, "--ref", serviceName}
 	command = append(command, options...)
@@ -88,8 +102,14 @@ func domainList(r *test.KnRunResultCollector, domainName string) {
 	assert.Check(r.T(), util.ContainsAll(out.Stdout, domainName))
 }
 
-func domainDescribe(r *test.KnRunResultCollector, domainName string) {
+func domainDescribe(r *test.KnRunResultCollector, domainName string, tls bool) {
 	out := r.KnTest().Kn().Run("domain", "describe", domainName)
 	r.AssertNoError(out)
-	assert.Assert(r.T(), util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Service"))
+	var url string
+	if tls {
+		url = "https://" + domainName
+	} else {
+		url = "http://" + domainName
+	}
+	assert.Assert(r.T(), util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Service", url))
 }