Repository: kubernetes-sigs/krew (go)

Issue:
creationTimestamp is null in all receipts
All my receipts have such a header:

```yaml
metadata:
   creationTimestamp: null
   name: krew
```

including `creationTimestamp`. This field carries no information and is annoying. There are two options:

* Get rid of this obsolete field.
* Fill the field when installing the plugin.

Following https://kubernetes.io/docs/reference/using-api/api-concepts/#generated-values, it seems most inline with k8s concepts, if the stored timestamp is the time when the plugin was first installed.

/help

Pull request:
Add creation timestamp to receipts on install
This PR adds the current time as the timestamp when a plugin is installed and leaves it alone when something is upgraded. I wasn't sure if we should do something about existing receipts with null `creationTimestamp`'s but figured it would be ok without something.

/assign @ahmetb 
/assign @corneliusweig 

Fixes #632 


Patch:
diff --git a/internal/installation/install.go b/internal/installation/install.go
index 24615da..71fbdaf 100644
--- a/internal/installation/install.go
+++ b/internal/installation/install.go
@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 
 	"sigs.k8s.io/krew/internal/download"
@@ -84,8 +85,9 @@ func Install(p environment.Paths, plugin index.Plugin, indexName string, opts In
 	}, opts); err != nil {
 		return errors.Wrap(err, "install failed")
 	}
+
 	klog.V(3).Infof("Storing install receipt for plugin %s", plugin.Name)
-	err = receipt.Store(receipt.New(plugin, indexName), p.PluginInstallReceiptPath(plugin.Name))
+	err = receipt.Store(receipt.New(plugin, indexName, metav1.Now()), p.PluginInstallReceiptPath(plugin.Name))
 	return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail")
 }
 
diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go
index 83a2735..3c1a0e7 100644
--- a/internal/installation/receipt/receipt.go
+++ b/internal/installation/receipt/receipt.go
@@ -18,6 +18,7 @@ import (
 	"io/ioutil"
 
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 
 	"sigs.k8s.io/krew/internal/index/indexscanner"
@@ -43,7 +44,8 @@ func Load(path string) (index.Receipt, error) {
 }
 
 // New returns a new receipt with the given plugin and index name.
-func New(plugin index.Plugin, indexName string) index.Receipt {
+func New(plugin index.Plugin, indexName string, timestamp metav1.Time) index.Receipt {
+	plugin.CreationTimestamp = timestamp
 	return index.Receipt{
 		Plugin: plugin,
 		Status: index.ReceiptStatus{
diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go
index bac6d32..711e977 100644
--- a/internal/installation/upgrade.go
+++ b/internal/installation/upgrade.go
@@ -78,7 +78,7 @@ func Upgrade(p environment.Paths, plugin index.Plugin, indexName string) error {
 	}
 
 	klog.V(2).Infof("Upgrading install receipt for plugin %s", plugin.Name)
-	if err = receipt.Store(receipt.New(plugin, indexName), p.PluginInstallReceiptPath(plugin.Name)); err != nil {
+	if err = receipt.Store(receipt.New(plugin, indexName, installReceipt.CreationTimestamp), p.PluginInstallReceiptPath(plugin.Name)); err != nil {
 		return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail")
 	}
 


Test patch:
diff --git a/integration_test/install_test.go b/integration_test/install_test.go
index 8004603..a5d6c03 100644
--- a/integration_test/install_test.go
+++ b/integration_test/install_test.go
@@ -21,6 +21,7 @@ import (
 	"strings"
 	"testing"
 
+	"sigs.k8s.io/krew/internal/environment"
 	"sigs.k8s.io/krew/pkg/constants"
 )
 
@@ -44,6 +45,11 @@ func TestKrewInstall(t *testing.T) {
 	test.Krew("install", validPlugin).RunOrFailOutput()
 	test.AssertExecutableInPATH("kubectl-" + validPlugin)
 	test.AssertPluginFromIndex(validPlugin, "default")
+
+	receiptPath := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
+	if r := test.loadReceipt(receiptPath); r.CreationTimestamp.Time.IsZero() {
+		t.Fatal("expected receipt to have a valid creationTimestamp")
+	}
 }
 
 func TestKrewInstallReRun(t *testing.T) {
diff --git a/integration_test/testutil_test.go b/integration_test/testutil_test.go
index 102e351..9bd1100 100644
--- a/integration_test/testutil_test.go
+++ b/integration_test/testutil_test.go
@@ -34,6 +34,7 @@ import (
 	"sigs.k8s.io/krew/internal/installation/receipt"
 	"sigs.k8s.io/krew/internal/testutil"
 	"sigs.k8s.io/krew/pkg/constants"
+	"sigs.k8s.io/krew/pkg/index"
 )
 
 const (
@@ -161,10 +162,7 @@ func (it *ITest) AssertPluginFromIndex(plugin, indexName string) {
 	it.t.Helper()
 
 	receiptPath := environment.NewPaths(it.Root()).PluginInstallReceiptPath(plugin)
-	r, err := receipt.Load(receiptPath)
-	if err != nil {
-		it.t.Fatalf("error loading receipt: %v", err)
-	}
+	r := it.loadReceipt(receiptPath)
 	if r.Status.Source.Name != indexName {
 		it.t.Errorf("wanted index '%s', got: '%s'", indexName, r.Status.Source.Name)
 	}
@@ -268,6 +266,14 @@ func (it *ITest) TempDir() *testutil.TempDir {
 	return it.tempDir
 }
 
+func (it *ITest) loadReceipt(path string) index.Receipt {
+	pluginReceipt, err := receipt.Load(path)
+	if err != nil {
+		it.t.Fatalf("error loading receipt: %v", err)
+	}
+	return pluginReceipt
+}
+
 // InitializeIndex initializes the krew index in `$root/index` with the actual krew-index.
 // It caches the index tree as in-memory tar after the first run.
 func (it *ITest) initializeIndex() {
diff --git a/integration_test/upgrade_test.go b/integration_test/upgrade_test.go
index b9cbffe..c427466 100644
--- a/integration_test/upgrade_test.go
+++ b/integration_test/upgrade_test.go
@@ -45,8 +45,9 @@ func TestKrewUpgrade(t *testing.T) {
 
 	// plugins installed via manifest get the special "detached" index so this needs to
 	// be changed to default in order for it to be upgraded here
-	receipt := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
-	modifyReceiptIndex(t, receipt, "default")
+	receiptPath := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
+	modifyReceiptIndex(t, receiptPath, "default")
+	initialCreationTimestamp := test.loadReceipt(receiptPath).CreationTimestamp
 
 	initialLocation := resolvePluginSymlink(test, validPlugin)
 	test.Krew("upgrade").RunOrFail()
@@ -54,6 +55,11 @@ func TestKrewUpgrade(t *testing.T) {
 	if initialLocation == eventualLocation {
 		t.Errorf("Expecting the plugin path to change but was the same.")
 	}
+
+	eventualCreationTimestamp := test.loadReceipt(receiptPath).CreationTimestamp
+	if initialCreationTimestamp != eventualCreationTimestamp {
+		t.Errorf("expected the receipt creationTimestamp to remain unchanged after upgrade")
+	}
 }
 
 func TestKrewUpgradePluginsFromCustomIndex(t *testing.T) {
@@ -64,14 +70,14 @@ func TestKrewUpgradePluginsFromCustomIndex(t *testing.T) {
 	test.WithDefaultIndex().WithCustomIndexFromDefault("foo")
 	test.Krew("install", "foo/"+validPlugin).RunOrFail()
 
-	receipt := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
-	modifyManifestVersion(t, receipt, "v0.0.0")
+	receiptPath := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
+	modifyManifestVersion(t, receiptPath, "v0.0.0")
 	out := string(test.Krew("upgrade").RunOrFailOutput())
 	if !strings.Contains(out, "Upgrading plugin: foo/"+validPlugin) {
 		t.Errorf("expected plugin foo/%s to be upgraded", validPlugin)
 	}
 
-	modifyManifestVersion(t, receipt, "v0.0.0")
+	modifyManifestVersion(t, receiptPath, "v0.0.0")
 	out = string(test.Krew("upgrade", validPlugin).RunOrFailOutput())
 	if !strings.Contains(out, "Upgrading plugin: foo/"+validPlugin) {
 		t.Errorf("expected plugin foo/%s to be upgraded", validPlugin)
@@ -101,8 +107,8 @@ func TestKrewUpgradeNoSecurityWarningForCustomIndex(t *testing.T) {
 	test.WithDefaultIndex().WithCustomIndexFromDefault("foo")
 	test.Krew("install", "foo/"+validPlugin).RunOrFail()
 
-	receipt := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
-	modifyManifestVersion(t, receipt, "v0.0.1")
+	pluginReceipt := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
+	modifyManifestVersion(t, pluginReceipt, "v0.0.1")
 	out := string(test.Krew("upgrade").RunOrFailOutput())
 	if strings.Contains(out, "Run them at your own risk") {
 		t.Errorf("expected install of custom plugin to not show security warning: %v", out)
diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go
index ecb7e8e..4f0ac08 100644
--- a/internal/installation/receipt/receipt_test.go
+++ b/internal/installation/receipt/receipt_test.go
@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"sigs.k8s.io/krew/internal/index/indexscanner"
 	"sigs.k8s.io/krew/internal/testutil"
@@ -72,10 +73,12 @@ func TestLoad_preservesNonExistsError(t *testing.T) {
 }
 
 func TestNew(t *testing.T) {
+	timestamp := metav1.Now()
 	testPlugin := testutil.NewPlugin().WithName("foo").WithPlatforms(testutil.NewPlatform().V()).V()
 	wantReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V()
+	wantReceipt.CreationTimestamp = timestamp
 
-	gotReceipt := New(testPlugin, constants.DefaultIndexName)
+	gotReceipt := New(testPlugin, constants.DefaultIndexName, timestamp)
 	if diff := cmp.Diff(gotReceipt, wantReceipt); diff != "" {
 		t.Fatalf("expected receipts to match: %s", diff)
 	}