stacks: add host truncation examples

Signed-off-by: Marques Johansson <marques@upbound.io>

docs/troubleshoot.md

@@ -135,13 +135,78 @@ kubectl patch workloads.compute.crossplane.io test-workload -p '{"metadata":{"fi
 
 ## Host-Aware Resource Debugging
 
-Stack resources (including the Stack, service accounts, deployments, and jobs) are usually easy to identify by name. These resource names are based on the name used in the StackInstall or Stack resource.
-
-In some cases, the full name of a Stack resource, which could be up to 253 characters long, can not be represented in the created resources. For example, jobs and deployment names may not exceed 63 characters because these names are turned into resource label values which impose a 63 character limit. Stack created resources whose names would otherwise not be permitted in the Kubernetes API will be truncated with a unique suffix.
-
-When running the Stack Manager in host-aware mode, tenant stack resources created in the host controller namespace generally reuse the Stack names: "{tenant namespace}.{tenant name}".  In order to avoid the name length restrictions, these resources may be truncated at either or both of the namespace segment or over the entire name.  In these cases resource labels, owner references, and annotations should be consulted to identify the responsible Stack.
+Stack resources (including the Stack, service accounts, deployments, and jobs)
+are usually easy to identify by name. These resource names are based on the name
+used in the StackInstall or Stack resource.
+
+In some cases, the full name of a Stack resource, which could be up to 253
+characters long, can not be represented in the created resources. For example,
+jobs and deployment names may not exceed 63 characters because these names are
+turned into resource label values which impose a 63 character limit. Stack
+created resources whose names would otherwise not be permitted in the Kubernetes
+API will be truncated with a unique suffix.
+
+When running the Stack Manager in host-aware mode, tenant stack resources
+created in the host controller namespace generally reuse the Stack names:
+"{tenant namespace}.{tenant name}".  In order to avoid the name length
+restrictions, these resources may be truncated at either or both of the
+namespace segment or over the entire name.  In these cases resource labels,
+owner references, and annotations should be consulted to identify the
+responsible Stack.
 
 * [Relationship Labels](https://github.com/crossplane/crossplane/blob/master/design/one-pager-stack-relationship-labels.md)
 * [Owner References](https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#owners-and-dependents)
-* Annotations: `tenant.crossplane.io/{singular}-name` and `tenant.crossplane.io/{singular}-namespace` (_singular_ may be `stackinstall`, `clusterstackinstall` or `stack`)
+* Annotations: `tenant.crossplane.io/{singular}-name` and
+  `tenant.crossplane.io/{singular}-namespace` (_singular_ may be `stackinstall`,
+  `clusterstackinstall` or `stack`)
+
+### Example
+
+Long resource names may be present on the tenant.
+
+```console
+$ name=stack-with-a-really-long-resource-name-so-long-that-it-will-be-truncated
+$ ns=this-is-just-a-really-long-namespace-name-at-the-character-max
+$ kubectl create ns $ns
+$ kubectl crossplane stack install --namespace $ns crossplane/sample-stack-wordpress:0.1.1 $name
+```
+
+When used as host resource names, the stack namespace and stack are concatenated
+ to form host names, as well as labels.  These resource names and label values
+ must be truncated to fit the 63 character limit on label values.
 
+```console
+$ kubectl --context=crossplane-host -n tenant-controllers get job -o yaml
+apiVersion: v1
+items:
+- apiVersion: batch/v1
+  kind: Job
+  metadata:
+    annotations:
+      tenant.crossplane.io/stackinstall-name: stack-with-a-really-long-resource-name-so-long-that-it-will-be-truncated
+      tenant.crossplane.io/stackinstall-namespace: this-is-just-a-really-long-namespace-name-at-the-character-max
+    creationTimestamp: "2020-03-20T17:06:25Z"
+    labels:
+      core.crossplane.io/parent-group: stacks.crossplane.io
+      core.crossplane.io/parent-kind: StackInstall
+      core.crossplane.io/parent-name: stack-with-a-really-long-resource-name-so-long-that-it-wi-alqdw
+      core.crossplane.io/parent-namespace: this-is-just-a-really-long-namespace-name-at-the-character-max
+      core.crossplane.io/parent-uid: 596705e4-a28e-47c9-a907-d2732f07a85e
+      core.crossplane.io/parent-version: v1alpha1
+    name: this-is-just-a-really-long-namespace-name-at-the-characte-egoav
+    namespace: tenant-controllers
+  spec:
+    backoffLimit: 0
+    completions: 1
+    parallelism: 1
+    selector:
+      matchLabels:
+        controller-uid: 8f290bf2-8c91-494a-a76b-27c2ccb9e0a8
+    template:
+      metadata:
+        creationTimestamp: null
+        labels:
+          controller-uid: 8f290bf2-8c91-494a-a76b-27c2ccb9e0a8
+          job-name: this-is-just-a-really-long-namespace-name-at-the-characte-egoav
+  ...
+```

pkg/controller/stacks/hosted/config.go

@@ -27,13 +27,15 @@ import (
 )
 
 const (
-	// AnnotationTenantNameFmt with a resource singular applied provides the
+	// AnnotationTenantNameFmt with a CR `singular` name applied provides the
 	// annotation key used to identify tenant resources by name on the host side
+	// Example: tenant.crossplane.io/stackinstall-name
 	AnnotationTenantNameFmt = "tenant.crossplane.io/%s-name"
 
-	// AnnotationTenantNamespaceFmt with a resource singular applied provides
+	// AnnotationTenantNamespaceFmt with a CR `singular` name applied provides
 	// the annotation key used to identify tenant resources by namespace on the
 	// host side
+	// Example: tenant.crossplane.io/stack-namespace
 	AnnotationTenantNamespaceFmt = "tenant.crossplane.io/%s-namespace"
 
 	errMissingOption = "host aware mode activated but %s is not set"
@@ -70,11 +72,13 @@ func NewConfig(hostControllerNamespace, tenantAPIServiceHost, tenantAPIServicePo
 	}, nil
 }
 
-// ObjectReferenceOnHost maps object with given name and namespace into single
-// controller namespace on Host Cluster.
-// The resource name on the host cluster is truncated to label value length
-// because the name may be used in labels defined by an admission controller, as
-// is the case for jobs and deployments.
+// ObjectReferenceOnHost maps objects with a given name and namespace into a
+// single controller namespace on the Host Cluster.
+//
+// The resource name on the host cluster may be truncated from the original
+// tenant name to fit label value length.  The resource name may be used as a
+// label, as is the case for jobs and deployments where the admission controller
+// generates labels based on the resource name.
 func (c *Config) ObjectReferenceOnHost(name, namespace string) corev1.ObjectReference {
 	return corev1.ObjectReference{
 		Name:      truncate.LabelValue(fmt.Sprintf("%s.%s", namespace, name)),
@@ -83,7 +87,13 @@ func (c *Config) ObjectReferenceOnHost(name, namespace string) corev1.ObjectRefe
 }
 
 // ObjectReferenceAnnotationsOnHost returns a map for use as annotations on the
-// host to identify the named tenant resource
+// host to identify the named tenant resource. This annotation is used for
+// reference purposes to define a relationship to a single resource of a
+// specific kind. For example, this could be used to declare the tenant
+// stackinstall resource that is related to a host install job.
+//
+// On a host the original tenant resource name may be truncated away.
+// Annotations provide a way to store the original name without truncation.
 func ObjectReferenceAnnotationsOnHost(singular, name, namespace string) map[string]string {
 	nameLabel := fmt.Sprintf(AnnotationTenantNameFmt, singular)
 	nsLabel := fmt.Sprintf(AnnotationTenantNamespaceFmt, singular)

pkg/controller/stacks/install/stackinstall_test.go

@@ -790,7 +790,7 @@ func Test_stackInstallHandler_deleteOrphanedCRDs(t *testing.T) {
 	var (
 		nsLabel = fmt.Sprintf(stacks.LabelNamespaceFmt, namespace)
 
-		// MultiParentLabels refer to a stack name. while resource() is a
+		// MultiParentLabels refer to a stack name. While resource() is a
 		// stackinstall, it is an object that has a name that matches the stack
 		label = stacks.MultiParentLabel(resource())
 	)

pkg/controller/stacks/stack/stack.go

@@ -724,7 +724,8 @@ func (h *stackHandler) prepareDeployment(d *apps.Deployment) {
 
 	// force the deployment to use stack opinionated names and service account
 	suffix := "-controller"
-	name, _ := truncate.Truncate(h.ext.Name, truncate.LabelValueLength-len(suffix), truncate.DefaultSuffixLength)
+	size := truncate.LabelValueLength - len(suffix)
+	name, _ := truncate.Truncate(h.ext.Name, size, truncate.DefaultSuffixLength)
 	name += suffix
 	matchLabels := map[string]string{"app": name}
 	labels := stacks.ParentLabels(h.ext)

pkg/stacks/relationship.go

@@ -38,16 +38,24 @@ const (
 
 	LabelMultiParentPrefix = "parent.stacks.crossplane.io/"
 
-	LabelMultiParentNSFormat = "parent.stacks.crossplane.io/%s"
+	LabelMultiParentNSFormat = LabelMultiParentPrefix + "%s"
 
 	// LabelMultiParentFormat defines the format for combining a
 	// LabelMultiParentNSFormat with a named resource
+	// Example:
+	// fmt.Sprintf(LabelMultiParentFormat,
+	//   fmt.Sprintf(LabelMultiParentNSFormat,
+	//   nsName,
+	// ), resourceName)
 	LabelMultiParentFormat = "%s-%s"
 
 	// preserveNSLength is the number of characters using the label name that
 	// will be dedicated to identifying the namespace. This length will include
-	// truncation characters if the namespace exceeds this length.
-	// example: parent.stacks.crossplane.io/{up to 32 chars of NS}-{Name}
+	// truncation characters if the namespace exceeds this length. example:
+	// parent.stacks.crossplane.io/{up to 32 chars of NS}-{Name}
+	//
+	// NOTE: Changes to this length will prevent resources from be discovered
+	// and could lead to the deletion or recreation of resources.
 	preserveNSLength = 32
 )
 
@@ -69,6 +77,12 @@ type KindlyIdentifier interface {
 // namespace exceeds 32 characters. This truncation length permits another 32
 // characters for a (potentially truncated) resource name to be appended to the
 // label.
+//
+// Example: MultiParentLabelPrefix(resource.SetNamespace("foo")) ->
+//   "parent.stacks.crossplane.io/foo"
+//
+// A namespace name over 32 characters will be truncated in the returned label
+// prefix.
 func MultiParentLabelPrefix(stackParent metav1.Object) string {
 	ns := stackParent.GetNamespace()
 
@@ -83,10 +97,17 @@ func MultiParentLabelPrefix(stackParent metav1.Object) string {
 // The label returned is based on the MultiParentLabelPrefix, which may include
 // a truncation suffix, and is then potentially truncated again to fit in the
 // complete label length restrictions.
+//
+// Example: MultiParentLabel(resource.SetNamespace("foo").SetName("bar").) ->
+//   "parent.stacks.crossplane.io/foo-bar"
+//
+// A namespace name over 32 characters will be truncated in the returned label
+// prefix, if the namespace and name, combined exceed 63 characters an
+// additional truncation will be included.
 func MultiParentLabel(stackParent metav1.Object) string {
 	prefix := MultiParentLabelPrefix(stackParent)
 
-	// guaranteed 2 parts based on LabelMultiParentNSFormat
+	// guaranteed at least 2 parts based on LabelMultiParentNSFormat
 	prefixParts := strings.SplitN(prefix, "/", 2)
 
 	n := stackParent.GetName()

pkg/stacks/relationship_test.go

@@ -121,10 +121,7 @@ func TestMultiParentLabel(t *testing.T) {
 		{
 			name:        "TruncatedNS",
 			stackParent: resource(sixtyThreeAs+"z", resourceName, uidString),
-			// notice ukl4i is also the suffix with 64 a's because the sum is
-			// generated from the whole string. In the NS prefix only 27
-			// characters from the original are preserved.
-			want: "parent.stacks.crossplane.io/aaaaaaaaaaaaaaaaaaaaaaaaaa-ukl4i-" + resourceName,
+			want:        "parent.stacks.crossplane.io/aaaaaaaaaaaaaaaaaaaaaaaaaa-ukl4i-" + resourceName,
 		},
 		{
 			name:        "NotTruncatedOnNameLength",

pkg/stacks/truncate/truncate.go

@@ -23,7 +23,7 @@ import (
 	// sha1 is not cryptographically secure, but that is not a goal. we require
 	// predictable and uniform text transformation. Any checksum function with
 	// even distribution would suffice.
-	"crypto/sha1" // nolint:blacklist
+	"crypto/sha1" // nolint:gosec
 	"encoding/base32"
 	"fmt"
 	"strings"
@@ -51,9 +51,19 @@ const (
 // Truncate replaces the suffixLength number of trailing characters from str
 // with a consistent hash fragment based on that string. The suffix will include
 // a leading hyphen.
+//
 // An error will be returned if the truncation length is less than the
 // suffixLength, or truncation length is greater than sha1Length, or the suffix
 // length is less than 2.
+//
+// Example:
+// If the base32 sum of a digest of "aaaaaaaaaaa" is "ovo", with a suffix length of 4:
+//
+// Truncating this string to a length of 11 would result in "aaaaaaaaaaa"
+// Truncating this string to a length of 8 would result in "aaaa-ovo"
+// Truncating this string to a length of 7 would result in "aaa-ovo"
+// Truncating this string to a length of 5 would result in "a-ovo"
+// Truncating this string to a length of 4 would result in "-ovo"
 func Truncate(str string, length, suffixLength int) (string, error) {
 	if len(str) <= length {
 		return str, nil