diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml new file mode 100644 index 00000000..92454b39 --- /dev/null +++ b/.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 \ No newline at end of file diff --git a/.gitignore b/.gitignore index 26a28f3c..e26a85f5 100644 --- a/.gitignore +++ b/.gitignore @@ -28,4 +28,7 @@ __debug* ### VisualStudioCode Patch ### # Ignore all local history of files -**/.history \ No newline at end of file +**/.history + +### Jetbrains IDEs ### +.idea/* \ No newline at end of file diff --git a/.vscode/launch.json b/.vscode/launch.json index 980b6f76..443c4e4e 100644 --- a/.vscode/launch.json +++ b/.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", diff --git a/default-go-lint.mk b/default-go-lint.mk index 216786f2..b952ce2b 100644 --- a/default-go-lint.mk +++ b/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 diff --git a/internal/kpt/fnruntime/container.go b/internal/kpt/fnruntime/container.go index 74506278..75aa24d6 100644 --- a/internal/kpt/fnruntime/container.go +++ b/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 diff --git a/internal/kpt/fnruntime/runner.go b/internal/kpt/fnruntime/runner.go index 8fcea9c5..3ea60d13 100644 --- a/internal/kpt/fnruntime/runner.go +++ b/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 diff --git a/internal/kpt/fnruntime/runner_test.go b/internal/kpt/fnruntime/runner_test.go index 176b86b2..ecbb0e38 100644 --- a/internal/kpt/fnruntime/runner_test.go +++ b/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 +} diff --git a/internal/kpt/util/get/get.go b/internal/kpt/util/get/get.go index 953581d8..1901b4a2 100644 --- a/internal/kpt/util/get/get.go +++ b/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 } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index e1c38b40..8af03a9c 100644 --- a/pkg/apiserver/apiserver.go +++ b/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 } diff --git a/pkg/apiserver/webhooks_test.go b/pkg/apiserver/webhooks_test.go index 60f804d1..afe85f2e 100644 --- a/pkg/apiserver/webhooks_test.go +++ b/pkg/apiserver/webhooks_test.go @@ -133,7 +133,7 @@ func captureStderr(f func()) string { return out } -func TestWatchCertificates(t *testing.T) { +func TestWatchCertificatesInvalidDirectory(t *testing.T) { // method for processing klog output assertLogMessages := func(log string) error { if len(log) > 0 { @@ -163,50 +163,152 @@ func TestWatchCertificates(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - // firstly test error occuring from invalid entity for watcher to watch. aka invalid dir, err expected - go watchCertificates(ctx, "Dummy Directory that does not exist", certFile, keyFile) - invalid_watch_entity_logs := captureStderr(func() { - time.Sleep(1 * time.Second) // Give some time for the logs to be flushed + // firstly test error occurring from invalid entity for watcher to watch. aka invalid dir, err expected + go watchCertificates(ctx, "/random-dir-that-does-not-exist", certFile, keyFile) + time.Sleep(5 * time.Second) // Give some time for the logs to be flushed }) t.Log(invalid_watch_entity_logs) err = assertLogMessages(invalid_watch_entity_logs) require.Error(t, err) +} - go watchCertificates(ctx, webhookCfg.CertStorageDir, certFile, keyFile) - time.Sleep(1 * time.Second) +func TestWatchCertificatesSuccessfulReload(t *testing.T) { + // method for processing klog output + assertLogMessages := func(log string) error { + if len(log) > 0 { + if log[0] == 'E' || log[0] == 'W' || log[0] == 'F' { + return errors.New("Error Occured in Watcher") + } + } + return nil + } + // Set up the temp directory with dummy certificate files + webhookCfg := WebhookConfig{ + Type: WebhookTypeUrl, + Host: "localhost", + CertStorageDir: t.TempDir(), + } + defer func() { + require.NoError(t, os.RemoveAll(webhookCfg.CertStorageDir)) + }() - //create file to trigger change but not alter the certificate contents - //should trigger reload and certificate reloaded successfully - newFilePath := filepath.Join(webhookCfg.CertStorageDir, "new_temp_file.txt") - _, err = os.Create(newFilePath) + _, err := createCerts(&webhookCfg) require.NoError(t, err) + keyFile := filepath.Join(webhookCfg.CertStorageDir, "tls.key") + certFile := filepath.Join(webhookCfg.CertStorageDir, "tls.crt") + + // Create a context with a cancel function + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + valid_reload_logs := captureStderr(func() { - time.Sleep(1 * time.Second) // Give some time for the logs to be flushed + go watchCertificates(ctx, webhookCfg.CertStorageDir, certFile, keyFile) + // give some time for watchCertificates method to spin up + time.Sleep(3 * time.Second) + + //create file to trigger change but not alter the certificate contents + //should trigger reload and certificate reloaded successfully + newFilePath := filepath.Join(webhookCfg.CertStorageDir, "new_temp_file.txt") + _, err = os.Create(newFilePath) + require.NoError(t, err) + + time.Sleep(5 * time.Second) // Give some time for the logs to be flushed }) t.Log(valid_reload_logs) err = assertLogMessages(valid_reload_logs) require.NoError(t, err) +} - // Modify the certificate file to trigger a file system event - // should cause an error log since cert contents are not valid anymore - certModTime = time.Time{} - err = os.WriteFile(certFile, []byte("dummy text"), 0660) +func TestWatchCertificatesInvalidCertReload(t *testing.T) { + + // method for processing klog output + assertLogMessages := func(log string) error { + if len(log) > 0 { + if log[0] == 'E' || log[0] == 'W' || log[0] == 'F' { + return errors.New("Error Occured in Watcher") + } + } + return nil + } + // Set up the temp directory with dummy certificate files + webhookCfg := WebhookConfig{ + Type: WebhookTypeUrl, + Host: "localhost", + CertStorageDir: t.TempDir(), + } + defer func() { + require.NoError(t, os.RemoveAll(webhookCfg.CertStorageDir)) + }() + + _, err := createCerts(&webhookCfg) require.NoError(t, err) + keyFile := filepath.Join(webhookCfg.CertStorageDir, "tls.key") + certFile := filepath.Join(webhookCfg.CertStorageDir, "tls.crt") + + // Create a context with a cancel function + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + invalid_reload_logs := captureStderr(func() { - time.Sleep(1 * time.Second) + go watchCertificates(ctx, webhookCfg.CertStorageDir, certFile, keyFile) + // give some time for watchCertificates method to spin up + time.Sleep(3 * time.Second) + + // Modify the certificate file to trigger a file system event + // should cause an error log since cert contents are not valid anymore + certModTime = time.Time{} + err = os.WriteFile(certFile, []byte("dummy text"), 0660) + require.NoError(t, err) + + time.Sleep(5 * time.Second) }) t.Log(invalid_reload_logs) err = assertLogMessages(invalid_reload_logs) require.Error(t, err) +} + +func TestWatchCertificatesGracefulTermination(t *testing.T) { + // method for processing klog output + assertLogMessages := func(log string) error { + if len(log) > 0 { + if log[0] == 'E' || log[0] == 'W' || log[0] == 'F' { + return errors.New("Error Occured in Watcher") + } + } + return nil + } + // Set up the temp directory with dummy certificate files + webhookCfg := WebhookConfig{ + Type: WebhookTypeUrl, + Host: "localhost", + CertStorageDir: t.TempDir(), + } + defer func() { + require.NoError(t, os.RemoveAll(webhookCfg.CertStorageDir)) + }() + + _, err := createCerts(&webhookCfg) + require.NoError(t, err) + + keyFile := filepath.Join(webhookCfg.CertStorageDir, "tls.key") + certFile := filepath.Join(webhookCfg.CertStorageDir, "tls.crt") + + // Create a context with a cancel function + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - go watchCertificates(ctx, webhookCfg.CertStorageDir, certFile, keyFile) - // trigger graceful termination - cancel() graceful_termination_logs := captureStderr(func() { - time.Sleep(1 * time.Second) + go watchCertificates(ctx, webhookCfg.CertStorageDir, certFile, keyFile) + // give some time for watchCertificates method to spin up + time.Sleep(3 * time.Second) + + // trigger graceful termination + cancel() + + time.Sleep(5 * time.Second) }) t.Log(graceful_termination_logs) err = assertLogMessages(graceful_termination_logs) diff --git a/pkg/cache/memory/cache_test.go b/pkg/cache/memory/cache_test.go index fb130d2b..ed8dd323 100644 --- a/pkg/cache/memory/cache_test.go +++ b/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) } diff --git a/pkg/cache/memory/repository.go b/pkg/cache/memory/repository.go index d584a2d0..b50e511e 100644 --- a/pkg/cache/memory/repository.go +++ b/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, } diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index a7588adb..27ec82b8 100644 --- a/pkg/cmd/server/start.go +++ b/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.") diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 0f6ab2dd..75272209 100644 --- a/pkg/engine/engine.go +++ b/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, @@ -328,7 +329,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, diff --git a/pkg/git/package.go b/pkg/git/package.go index be203b4d..64ff3151 100644 --- a/pkg/git/package.go +++ b/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 } diff --git a/pkg/kpt/fs_test.go b/pkg/kpt/fs_test.go index 6a3d5dc4..b822ec1e 100644 --- a/pkg/kpt/fs_test.go +++ b/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()) diff --git a/pkg/kpt/render_test.go b/pkg/kpt/render_test.go index f10288fd..0eb77dcd 100644 --- a/pkg/kpt/render_test.go +++ b/pkg/kpt/render_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/nephio-project/porch/internal/kpt/fnruntime" "github.com/nephio-project/porch/internal/kpt/util/render" "github.com/nephio-project/porch/pkg/kpt/printer/fake" "sigs.k8s.io/kustomize/kyaml/filesys" @@ -65,7 +66,7 @@ func TestRender(t *testing.T) { FileSystem: filesys.FileSystemOrOnDisk{}, Output: &output, } - r.RunnerOptions.InitDefaults() + r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix) if _, err := r.Execute(fake.CtxWithDefaultPrinter()); err != nil { t.Errorf("Render failed: %v", err) diff --git a/pkg/meta/fake/memorystore.go b/pkg/meta/fake/memorystore.go index a3dd72a3..dd48e66d 100644 --- a/pkg/meta/fake/memorystore.go +++ b/pkg/meta/fake/memorystore.go @@ -20,6 +20,7 @@ import ( configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" "github.com/nephio-project/porch/pkg/meta" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" ) @@ -27,28 +28,28 @@ import ( // MemoryMetadataStore is an in-memory implementation of the MetadataStore interface. It // means metadata about packagerevisions will be stored in memory, which is useful for testing. type MemoryMetadataStore struct { - Metas []meta.PackageRevisionMeta + Metas []metav1.ObjectMeta } var _ meta.MetadataStore = &MemoryMetadataStore{} -func (m *MemoryMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) { for _, meta := range m.Metas { if meta.Name == namespacedName.Name && meta.Namespace == namespacedName.Namespace { return meta, nil } } - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, namespacedName.Name, ) } -func (m *MemoryMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) { return m.Metas, nil } -func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta, repoName string, pkgRevUID types.UID) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) { for _, m := range m.Metas { if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { return m, apierrors.NewAlreadyExists( @@ -61,7 +62,7 @@ func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta meta.Packag return pkgRevMeta, nil } -func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) { i := -1 for j, m := range m.Metas { if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { @@ -69,7 +70,7 @@ func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.Packag } } if i < 0 { - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.porch.kpt.dev", Resource: "packagerevisions"}, pkgRevMeta.Name, ) @@ -78,10 +79,10 @@ func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.Packag return pkgRevMeta, nil } -func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, _ bool) (meta.PackageRevisionMeta, error) { - var metas []meta.PackageRevisionMeta +func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, _ bool) (metav1.ObjectMeta, error) { + var metas []metav1.ObjectMeta found := false - var deletedMeta meta.PackageRevisionMeta + var deletedMeta metav1.ObjectMeta for _, m := range m.Metas { if m.Name == namespacedName.Name && m.Namespace == namespacedName.Namespace { found = true @@ -91,7 +92,7 @@ func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.N } } if !found { - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, namespacedName.Name, ) diff --git a/pkg/meta/store.go b/pkg/meta/store.go index bbb6e402..6a172c78 100644 --- a/pkg/meta/store.go +++ b/pkg/meta/store.go @@ -45,23 +45,11 @@ var ( // examples of metadata we want to keep is labels, annotations, owner references, and // finalizers. type MetadataStore interface { - Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) - List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) - Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repoName string, pkgRevUID types.UID) (PackageRevisionMeta, error) - Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) - Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (PackageRevisionMeta, error) -} - -// PackageRevisionMeta contains metadata about a specific PackageRevision. The -// PackageRevision is identified by the name and namespace. -type PackageRevisionMeta struct { - Name string - Namespace string - Labels map[string]string - Annotations map[string]string - Finalizers []string - OwnerReferences []metav1.OwnerReference - DeletionTimestamp *metav1.Time + Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) + List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) + Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) + Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) + Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (metav1.ObjectMeta, error) } var _ MetadataStore = &crdMetadataStore{} @@ -78,20 +66,20 @@ type crdMetadataStore struct { coreClient client.Client } -func (c *crdMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Get", trace.WithAttributes()) defer span.End() var internalPkgRev internalapi.PackageRev err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) if err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) { +func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::List", trace.WithAttributes()) defer span.End() @@ -100,14 +88,14 @@ func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) if err != nil { return nil, err } - var pkgRevMetas []PackageRevisionMeta + var pkgRevMetas []metav1.ObjectMeta for _, ipr := range internalPkgRevList.Items { pkgRevMetas = append(pkgRevMetas, toPackageRevisionMeta(&ipr)) } return pkgRevMetas, nil } -func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repoName string, pkgRevUID types.UID) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Create", trace.WithAttributes()) defer span.End() @@ -141,12 +129,12 @@ func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta PackageRevisio if apierrors.IsAlreadyExists(err) { return c.Update(ctx, pkgRevMeta) } - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Update", trace.WithAttributes()) defer span.End() @@ -157,7 +145,7 @@ func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisio } err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) if err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } // Copy updated labels to the CR and add the repository label @@ -185,12 +173,12 @@ func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisio klog.Infof("Updating packagerev %s/%s", internalPkgRev.Namespace, internalPkgRev.Name) if err := c.coreClient.Update(ctx, &internalPkgRev); err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizers bool) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizers bool) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Delete", trace.WithAttributes()) defer span.End() @@ -210,17 +198,17 @@ func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.Name return nil }) if retriedErr != nil { - return PackageRevisionMeta{}, retriedErr + return metav1.ObjectMeta{}, retriedErr } klog.Infof("Deleting packagerev %s/%s", internalPkgRev.Namespace, internalPkgRev.Name) if err := c.coreClient.Delete(ctx, &internalPkgRev); err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisionMeta { +func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) metav1.ObjectMeta { labels := internalPkgRev.Labels delete(labels, PkgRevisionRepoLabel) @@ -232,6 +220,7 @@ func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisi ownerReferences = append(ownerReferences, or) } } + internalPkgRev.OwnerReferences = ownerReferences var finalizers []string for _, f := range internalPkgRev.Finalizers { @@ -240,15 +229,8 @@ func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisi } } - return PackageRevisionMeta{ - Name: internalPkgRev.Name, - Namespace: internalPkgRev.Namespace, - Labels: labels, - Annotations: internalPkgRev.Annotations, - Finalizers: finalizers, - OwnerReferences: ownerReferences, - DeletionTimestamp: internalPkgRev.DeletionTimestamp, - } + internalPkgRev.Finalizers = finalizers + return internalPkgRev.ObjectMeta } func isPackageRevOwnerRef(or metav1.OwnerReference, pkgRevName string) bool { diff --git a/pkg/oci/oci.go b/pkg/oci/oci.go index 0983dbc5..a9c6be4e 100644 --- a/pkg/oci/oci.go +++ b/pkg/oci/oci.go @@ -31,7 +31,6 @@ import ( configapi "github.com/nephio-project/porch/api/porchconfig/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" @@ -262,7 +261,7 @@ type ociPackageRevision struct { created time.Time resourceVersion string uid types.UID - metadata meta.PackageRevisionMeta + metadata metav1.ObjectMeta parent *ociRepository tasks []v1alpha1.Task @@ -425,10 +424,10 @@ func (p *ociPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.P return nil } -func (p *ociPackageRevision) GetMeta() meta.PackageRevisionMeta { +func (p *ociPackageRevision) GetMeta() metav1.ObjectMeta { return p.metadata } -func (p *ociPackageRevision) SetMeta(metadata meta.PackageRevisionMeta) { +func (p *ociPackageRevision) SetMeta(metadata metav1.ObjectMeta) { p.metadata = metadata } diff --git a/pkg/repository/fake/packagerevision.go b/pkg/repository/fake/packagerevision.go index 8f962065..bd271d67 100644 --- a/pkg/repository/fake/packagerevision.go +++ b/pkg/repository/fake/packagerevision.go @@ -19,8 +19,8 @@ import ( "github.com/nephio-project/porch/api/porch/v1alpha1" 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" ) @@ -91,9 +91,9 @@ func (f *FakePackageRevision) UpdateLifecycle(context.Context, v1alpha1.PackageR return nil } -func (f *FakePackageRevision) GetMeta() meta.PackageRevisionMeta { - return meta.PackageRevisionMeta{} +func (f *FakePackageRevision) GetMeta() metav1.ObjectMeta { + return metav1.ObjectMeta{} } -func (f *FakePackageRevision) SetMeta(meta.PackageRevisionMeta) { +func (f *FakePackageRevision) SetMeta(metav1.ObjectMeta) { } diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index 819f5249..543dd0ac 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -21,7 +21,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/nephio-project/porch/api/porch/v1alpha1" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" - "github.com/nephio-project/porch/pkg/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -100,10 +100,10 @@ type PackageRevision interface { ToMainPackageRevision() PackageRevision // Get the Kubernetes metadata for the package revision - GetMeta() meta.PackageRevisionMeta + GetMeta() metav1.ObjectMeta // Set the Kubernetes metadata for the package revision - SetMeta(meta.PackageRevisionMeta) + SetMeta(metav1.ObjectMeta) } // Package is an abstract package. diff --git a/pkg/task/render_test.go b/pkg/task/render_test.go index 1d1fc17a..748b6260 100644 --- a/pkg/task/render_test.go +++ b/pkg/task/render_test.go @@ -31,7 +31,7 @@ import ( func TestRender(t *testing.T) { runnerOptions := fnruntime.RunnerOptions{} - runnerOptions.InitDefaults() + runnerOptions.InitDefaults(fnruntime.GCRImagePrefix) render := &renderPackageMutation{ runnerOptions: runnerOptions,