Merge branch 'main' into add_gosec_gh_action

.github/workflows/golangci-lint.yaml

@@ -0,0 +1,54 @@
+# Copyright 2024 The Nephio Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: golangci-lint
+on:
+  push:
+    paths-ignore:
+      - "docs/**"
+      - "release/**"
+      - ".prow.yaml"
+      - "OWNERS"
+  pull_request:
+    paths-ignore:
+      - "docs/**"
+      - "release/**"
+      - ".prow.yaml"
+      - "OWNERS"
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  # pull-requests: read
+
+env:
+  GO_VERSION: 1.22.0
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Porch
+        uses: actions/checkout@v4
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.62.2
+          args: --timeout=10m --go=${{ env.GO_VERSION }} --exclude-dirs=test --exclude-generated=true --issues-exit-code=0

.gitignore

@@ -31,4 +31,9 @@ __debug*
 **/.history
 
 # gosec artifacts
-*results.html
+*results.html
+=======
+
+### Jetbrains IDEs ###
+.idea/*
+

.vscode/launch.json

@@ -3,6 +3,12 @@
     // Hover to view descriptions of existing attributes.
     // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
     "version": "0.2.0",
+
+    // A configuration for running the porch server through vscode for the use of debugging.
+    // Assumes a cluster is set up alongside Git (./scripts/setup-dev-env.sh) and porch components (make ../porch/run-in-kind-no-server) are pre-configured
+    // With the configuration above one can use Vscode (Run & Debug) and launch server.
+    // This launches the porch server through vscode outside the cluster and the logs can be viewed in the debug console.
+    // Breakpoints can be added throughout the porch server code to debug.
     "configurations": [
         {
             "name": "Launch Server",
@@ -36,6 +42,21 @@
                 "ENABLE_PACKAGEVARIANTSETS": "true"
             }
         },
+        // A configuration for running a porchctl command using the vscode debugger.
+        // Assumes a cluster is set up alongside Git (scripts/setup-dev-env.sh) and porch components (make run-in-kind) in the /porch directory are pre-configured
+        // This allows for the running of porchctl commands through vscode outside the cluster and the logs can be viewed in the debug console.
+        // Breakpoints can be added throughout the porch server code to debug.
+        {
+            "name": "Run Porchctl command",
+            "type": "go",
+            "request": "launch",
+            "mode": "auto",
+            "program": "${workspaceFolder}/cmd/porchctl/main.go",
+            "args": [
+                "rpkg", "init", "porch-package-name", "--workspace=v1", "--namespace=repository-namespace", "--repository=repo-name"
+            ],
+            "cwd": "${workspaceFolder}"
+        },
         {
             "name": "Launch test function",
             "type": "go",

default-go-lint.mk

@@ -1,4 +1,4 @@
-#  Copyright 2023 The Nephio Authors.
+#  Copyright 2023-2024 The Nephio Authors.
 #
 #  Licensed under the Apache License, Version 2.0 (the "License");
 #  you may not use this file except in compliance with the License.
@@ -13,7 +13,8 @@
 #  limitations under the License.
 
 
-GOLANG_CI_VER ?= v1.57
+GOLANG_CI_VER ?= v1.62.2
+GO_VERSION ?= 1.22.0
 GIT_ROOT_DIR ?= $(dir $(lastword $(MAKEFILE_LIST)))
 include $(GIT_ROOT_DIR)/detect-container-runtime.mk
 
@@ -22,7 +23,7 @@ include $(GIT_ROOT_DIR)/detect-container-runtime.mk
 lint: ## Run Go linter against the codebase
 ifeq ($(CONTAINER_RUNNABLE), 0)
 	$(RUN_CONTAINER_COMMAND) docker.io/golangci/golangci-lint:${GOLANG_CI_VER}-alpine \
-	 golangci-lint run ./... -v --timeout 10m
+	 golangci-lint run ./... -v --go=${GO_VERSION} --timeout=10m --exclude-dirs=test --exclude-generated=true
 else
-	golangci-lint run ./... -v --timeout 10m
+	golangci-lint run ./... -v --go=${GO_VERSION} --timeout=10m --exclude-dirs=test --exclude-generated=true
 endif

internal/kpt/fnruntime/container.go

@@ -224,13 +224,22 @@ func NewContainerEnvFromStringSlice(envStr []string) *runtimeutil.ContainerEnv {
 	return ce
 }
 
-// ResolveToImageForCLI converts the function short path to the full image url.
-// If the function is Catalog function, it adds "gcr.io/kpt-fn/".e.g. set-namespace:v0.1 --> gcr.io/kpt-fn/set-namespace:v0.1
-func ResolveToImageForCLI(_ context.Context, image string) (string, error) {
-	if !strings.Contains(image, "/") {
-		return fmt.Sprintf("gcr.io/kpt-fn/%s", image), nil
+// ResolveToImageForCLIFunc returns a func that converts the KRM function short path to the full image url.
+// If the function is a catalog function, it prepends `prefix`, e.g. "set-namespace:v0.1" --> prefix + "set-namespace:v0.1".
+// A "/" is appended to `prefix` if it is not an empty string and does not end with a "/".
+func ResolveToImageForCLIFunc(prefix string) func(_ context.Context, image string) (string, error) {
+	prefix = strings.TrimSuffix(prefix, "/")
+	if prefix == "" {
+		return func(_ context.Context, image string) (string, error) {
+			return image, nil
+		}
+	}
+	return func(_ context.Context, image string) (string, error) {
+		if !strings.Contains(image, "/") {
+			return fmt.Sprintf("%s/%s", prefix, image), nil
+		}
+		return image, nil
 	}
-	return image, nil
 }
 
 // ContainerImageError is an error type which will be returned when

internal/kpt/fnruntime/runner.go

@@ -43,6 +43,7 @@ import (
 
 const (
 	FuncGenPkgContext = "builtins/gen-pkg-context"
+	GCRImagePrefix    = "gcr.io/kpt-fn/"
 )
 
 type RunnerOptions struct {
@@ -79,9 +80,9 @@ type RunnerOptions struct {
 // ImageResolveFunc is the type for a function that can resolve a partial image to a (more) fully-qualified name
 type ImageResolveFunc func(ctx context.Context, image string) (string, error)
 
-func (o *RunnerOptions) InitDefaults() {
+func (o *RunnerOptions) InitDefaults(defaultImagePrefix string) {
 	o.ImagePullPolicy = IfNotPresentPull
-	o.ResolveToImage = ResolveToImageForCLI
+	o.ResolveToImage = ResolveToImageForCLIFunc(defaultImagePrefix)
 }
 
 // NewRunner returns a FunctionRunner given a specification of a function

internal/kpt/fnruntime/runner_test.go

@@ -646,3 +646,34 @@ func TestPrintFnStderr(t *testing.T) {
 		})
 	}
 }
+
+func TestRunnerOptions_InitDefaults(t *testing.T) {
+	tests := map[string]struct {
+		prefix string
+	}{
+		"empty":             {prefix: ""},
+		"trailing_slash":    {prefix: "example.org/kpt-fn/"},
+		"no_trailing_slash": {prefix: "example.org/kpt-fn"},
+	}
+
+	const fnName = "my-krm-function"
+
+	for testName, tc := range tests {
+		t.Run(testName, func(t *testing.T) {
+			opts := &RunnerOptions{}
+			opts.InitDefaults(tc.prefix)
+
+			result, err := opts.ResolveToImage(context.TODO(), fnName)
+
+			assert.NoError(t, err)
+			assert.Equal(t, getExpectedPrefix(tc.prefix)+fnName, result)
+		})
+	}
+}
+
+func getExpectedPrefix(prefix string) string {
+	if prefix != "" && !strings.HasSuffix(prefix, "/") {
+		return prefix + "/"
+	}
+	return prefix
+}

internal/kpt/util/get/get.go

@@ -61,6 +61,10 @@ type Command struct {
 	// Kptfile. This determines how changes will be merged when updating the
 	// package.
 	UpdateStrategy kptfilev1.UpdateStrategyType
+
+	// DefaultKrmFunctionImagePrefix is the prefix to be used with unqualified
+	// KRM function image names. Defaults to "gcr.io/kpt-fn/".
+	DefaultKrmFunctionImagePrefix string
 }
 
 // Run runs the Command.
@@ -127,7 +131,7 @@ func (c Command) Run(ctx context.Context) error {
 		pr := printer.FromContextOrDie(ctx)
 		pr.Printf("\nCustomizing package for deployment.\n")
 		hookCmd := hook.Executor{}
-		hookCmd.RunnerOptions.InitDefaults()
+		hookCmd.RunnerOptions.InitDefaults(c.DefaultKrmFunctionImagePrefix)
 		hookCmd.PkgPath = c.Destination
 
 		builtinHooks := []kptfilev1.Function{
@@ -221,6 +225,10 @@ func (c *Command) DefaultValues() error {
 		c.UpdateStrategy = kptfilev1.ResourceMerge
 	}
 
+	if len(c.DefaultKrmFunctionImagePrefix) == 0 {
+		c.DefaultKrmFunctionImagePrefix = fnruntime.GCRImagePrefix
+	}
+
 	return nil
 }
 

pkg/apiserver/apiserver.go

@@ -237,7 +237,7 @@ func (c completedConfig) New() (*PorchServer, error) {
 
 	runnerOptionsResolver := func(namespace string) fnruntime.RunnerOptions {
 		runnerOptions := fnruntime.RunnerOptions{}
-		runnerOptions.InitDefaults()
+		runnerOptions.InitDefaults(c.ExtraConfig.DefaultImagePrefix)
 
 		return runnerOptions
 	}

pkg/cache/memory/cache_test.go

@@ -272,11 +272,11 @@ func createMetadataStoreFromArchive(t *testing.T, testPath, name string) meta.Me
 	}
 	if os.IsNotExist(err) {
 		return &fakemeta.MemoryMetadataStore{
-			Metas: []meta.PackageRevisionMeta{},
+			Metas: []metav1.ObjectMeta{},
 		}
 	}
 
-	var metas []meta.PackageRevisionMeta
+	var metas []metav1.ObjectMeta
 	if err := yaml.Unmarshal(c, &metas); err != nil {
 		t.Fatalf("Error unmarshalling metadata file for repository %s", name)
 	}

pkg/cache/memory/repository.go

@@ -28,6 +28,7 @@ import (
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/trace"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/klog/v2"
@@ -260,7 +261,7 @@ func (r *cachedRepository) createMainPackageRevision(ctx context.Context, update
 	// Create the package if it doesn't exist
 	_, err := r.metadataStore.Get(ctx, pkgRevMetaNN)
 	if errors.IsNotFound(err) {
-		pkgRevMeta := meta.PackageRevisionMeta{
+		pkgRevMeta := metav1.ObjectMeta{
 			Name:      updatedMain.KubeObjectName(),
 			Namespace: updatedMain.KubeObjectNamespace(),
 		}
@@ -426,7 +427,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
 		return nil, nil, err
 	}
 	// Create a map so we can quickly check if a specific PackageRevisionMeta exists.
-	existingPkgRevCRsMap := make(map[string]meta.PackageRevisionMeta)
+	existingPkgRevCRsMap := make(map[string]metav1.ObjectMeta)
 	for i := range existingPkgRevCRs {
 		pr := existingPkgRevCRs[i]
 		existingPkgRevCRsMap[pr.Name] = pr
@@ -497,7 +498,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
 	// a corresponding PackageRev CR.
 	for pkgRevName, pkgRev := range newPackageRevisionNames {
 		if _, found := existingPkgRevCRsMap[pkgRevName]; !found {
-			pkgRevMeta := meta.PackageRevisionMeta{
+			pkgRevMeta := metav1.ObjectMeta{
 				Name:      pkgRevName,
 				Namespace: r.repoSpec.Namespace,
 			}

pkg/cmd/server/start.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"time"
 
+	"github.com/nephio-project/porch/internal/kpt/fnruntime"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -244,7 +245,7 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) {
 	}
 
 	fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.")
-	fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", "gcr.io/kpt-fn/", "Default prefix for unqualified function names")
+	fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", fnruntime.GCRImagePrefix, "Default prefix for unqualified function names")
 	fs.StringVar(&o.CacheDirectory, "cache-directory", "", "Directory where Porch server stores repository and package caches.")
 	fs.IntVar(&o.MaxRequestBodySize, "max-request-body-size", 6*1024*1024, "Maximum size of the request body in bytes. Keep this in sync with function-runner's corresponding argument.")
 	fs.BoolVar(&o.UseGitCaBundle, "use-git-cabundle", false, "Determine whether to use a user-defined CaBundle for TLS towards git.")

pkg/engine/engine.go

@@ -30,6 +30,7 @@ import (
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/trace"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/klog/v2"
@@ -187,7 +188,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *
 	if err != nil {
 		return nil, err
 	}
-	pkgRevMeta := meta.PackageRevisionMeta{
+	pkgRevMeta := metav1.ObjectMeta{
 		Name:            repoPkgRev.KubeObjectName(),
 		Namespace:       repoPkgRev.KubeObjectNamespace(),
 		Labels:          obj.Labels,
@@ -325,7 +326,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string,
 }
 
 func (cad *cadEngine) updatePkgRevMeta(ctx context.Context, repoPkgRev repository.PackageRevision, apiPkgRev *api.PackageRevision) error {
-	pkgRevMeta := meta.PackageRevisionMeta{
+	pkgRevMeta := metav1.ObjectMeta{
 		Name:            repoPkgRev.KubeObjectName(),
 		Namespace:       repoPkgRev.KubeObjectNamespace(),
 		Labels:          apiPkgRev.Labels,

pkg/git/package.go

@@ -29,7 +29,6 @@ import (
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
 	"github.com/nephio-project/porch/internal/kpt/pkg"
 	kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
-	"github.com/nephio-project/porch/pkg/meta"
 	"github.com/nephio-project/porch/pkg/repository"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -51,7 +50,7 @@ type gitPackageRevision struct {
 	tree          plumbing.Hash       // Cached tree of the package itself, some descendent of commit.Tree()
 	commit        plumbing.Hash       // Current version of the package (commit sha)
 	tasks         []v1alpha1.Task
-	metadata      meta.PackageRevisionMeta
+	metadata      metav1.ObjectMeta
 }
 
 var _ repository.PackageRevision = &gitPackageRevision{}
@@ -318,11 +317,11 @@ func (p *gitPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.P
 	return p.repo.UpdateLifecycle(ctx, p, new)
 }
 
-func (p *gitPackageRevision) GetMeta() meta.PackageRevisionMeta {
+func (p *gitPackageRevision) GetMeta() metav1.ObjectMeta {
 	return p.metadata
 }
 
-func (p *gitPackageRevision) SetMeta(metadata meta.PackageRevisionMeta) {
+func (p *gitPackageRevision) SetMeta(metadata metav1.ObjectMeta) {
 	p.metadata = metadata
 }
 

pkg/kpt/fs_test.go

@@ -106,7 +106,7 @@ spec:
 		FileSystem: fs,
 		Runtime:    &runtime{},
 	}
-	r.RunnerOptions.InitDefaults()
+	r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 	r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull
 	_, err := r.Execute(fake.CtxWithDefaultPrinter())
 	if err != nil {
@@ -220,7 +220,7 @@ spec:
 		FileSystem: fs,
 		Runtime:    &runtime{},
 	}
-	r.RunnerOptions.InitDefaults()
+	r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 	r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull
 
 	_, err := r.Execute(fake.CtxWithDefaultPrinter())