From d5c69924df9c0a6282d77e62168cf65bd2eeafd0 Mon Sep 17 00:00:00 2001
From: justinsb <justinsb@google.com>
Date: Mon, 5 Feb 2024 08:58:14 -0500
Subject: [PATCH] Add developer hints

We've evolved a lot over time, and we could start nudging developers
to use the now-recommended approaches and avoid the less-recommended
approaches (without breaking compatability).

Introduce a "hints" package that enables this.
---
 pkg/hints/log.go                              | 33 +++++++++++++++++++
 .../declarative/pkg/applier/applylib.go       |  2 ++
 pkg/patterns/declarative/watch.go             |  2 ++
 3 files changed, 37 insertions(+)
 create mode 100644 pkg/hints/log.go

diff --git a/pkg/hints/log.go b/pkg/hints/log.go
new file mode 100644
index 00000000..8c511441
--- /dev/null
+++ b/pkg/hints/log.go
@@ -0,0 +1,33 @@
+package hints
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"k8s.io/klog/v2"
+)
+
+// EnvRecommendationsAreStrict is the environment variable that can be set
+// to make developer recommendations a panic.  It can be set by developers
+// while developing to help them keep up with changes to
+// kubebuilder-declarative-pattern and any upstream changes.
+const EnvRecommendationsAreStrict = "FAIL_ON_DEVELOPER_RECOMMENDATIONS"
+
+// DeveloperRecommendation will log a recommendation for developers;
+// helping developers keep up with changes here and upstream.
+// If EnvRecommendationsAreStrict is set, we will panic.
+func DeveloperRecommendation(ctx context.Context, msg string, keysAndValues ...any) {
+	log := klog.FromContext(ctx)
+	log.Info(msg, keysAndValues...)
+	if os.Getenv(EnvRecommendationsAreStrict) != "" {
+		s := fmt.Sprintf("recommendation for developers with %s environment variable set, so treating as a failure: %s", EnvRecommendationsAreStrict, msg)
+		for i := 0; (i + 1) < len(keysAndValues); i++ {
+			if i != 0 {
+				s += ", "
+			}
+			s += fmt.Sprintf("%v:%v", keysAndValues[i], keysAndValues[i+1])
+		}
+		panic(s)
+	}
+}
diff --git a/pkg/patterns/declarative/pkg/applier/applylib.go b/pkg/patterns/declarative/pkg/applier/applylib.go
index 5592e303..08cb1b22 100644
--- a/pkg/patterns/declarative/pkg/applier/applylib.go
+++ b/pkg/patterns/declarative/pkg/applier/applylib.go
@@ -12,6 +12,7 @@ import (
 	"k8s.io/client-go/dynamic"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/applylib/applyset"
+	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/hints"
 )
 
 type ApplysetOptions struct {
@@ -57,6 +58,7 @@ func (a *ApplySetApplier) Apply(ctx context.Context, opt ApplierOptions) error {
 
 	dynamicClient := opt.DynamicClient
 	if dynamicClient == nil {
+		hints.DeveloperRecommendation(context.Background(), "consider setting ApplierOptions.DynamicClient", "applierOptions", opt)
 		d, err := dynamic.NewForConfig(opt.RESTConfig)
 		if err != nil {
 			return fmt.Errorf("error building dynamic client: %w", err)
diff --git a/pkg/patterns/declarative/watch.go b/pkg/patterns/declarative/watch.go
index ec93bb3c..7e19f94a 100644
--- a/pkg/patterns/declarative/watch.go
+++ b/pkg/patterns/declarative/watch.go
@@ -33,6 +33,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/commonclient"
+	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/hints"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/watch"
 )
 
@@ -96,6 +97,7 @@ func WatchChildren(options WatchChildrenOptions) error {
 		httpClient = options.HTTPClient
 	} else {
 		if options.RESTConfig != nil {
+			hints.DeveloperRecommendation(context.Background(), "consider setting WatchChildrenOptions.HTTPClient, if setting RESTConfig", "options", options)
 			hc, err := rest.HTTPClientFor(options.RESTConfig)
 			if err != nil {
 				return err