Skip to content

Commit

Permalink
Disable ensureSameOrigin (#4000)
Browse files Browse the repository at this point in the history
* Disable ensureSameOrigin

* Default replay-strategy to false; remove ensureSameOrigin

* Remove same origin tests too

* Specify replay-strategy for this copy call

* Sort the created list to avoid flakes
  • Loading branch information
johnbelamaric authored Jul 13, 2023
1 parent f699982 commit 5ad4116
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 559 deletions.
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/copy/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner
Hidden: porch.HidePorchCommands,
}
r.Command.Flags().StringVar(&r.workspace, "workspace", "", "Workspace name of the copy of the package.")
r.Command.Flags().BoolVar(&r.replayStrategy, "replay-strategy", true, "Use replay strategy for creating new package revision.")
r.Command.Flags().BoolVar(&r.replayStrategy, "replay-strategy", false, "Use replay strategy for creating new package revision.")
return r
}

Expand Down
1 change: 1 addition & 0 deletions e2e/testdata/porch/rpkg-update/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ commands:
- copy
- --namespace=rpkg-update
- --workspace=update-3
- --replay-strategy=true
- git-3f036055f7ba68706372cbe0c4b14d553794f7c4
stdout: "git-7fcdd499f0790ac3bd8f805e3e5e00825641eb60 created\n"
- args:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package packagevariantset

import (
"context"
"sort"
"testing"

porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
Expand Down Expand Up @@ -135,6 +136,10 @@ func TestEnsurePackageVariants(t *testing.T) {
require.Equal(t, 1, len(fc.updated))
require.Equal(t, "my-pvs-dnrepo2-dnpkg2", fc.updated[0].GetName())
require.Equal(t, 2, len(fc.created))
// ordering of calls to create is not stable (map iteration)
sort.Slice(fc.created, func(i, j int) bool {
return fc.created[i].GetName() < fc.created[j].GetName()
})
require.Equal(t, "my-pvs-dnrepo3-dnpkg3", fc.created[0].GetName())
require.Equal(t, "my-pvs-dnrepo4-supersupersuperlooooooooooooooooooooooo-bec36506", fc.created[1].GetName())
}
110 changes: 0 additions & 110 deletions porch/pkg/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,6 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *
return nil, err
}

sameOrigin, err := ensureSameOrigin(ctx, repo, obj, revs)
if err != nil {
return nil, fmt.Errorf("error ensuring same origin: %w", err)
}

if !sameOrigin {
return nil, fmt.Errorf("cannot create revision of %s with a different origin than other package revisions in the same package", obj.Spec.PackageName)
}

draft, err := repo.CreatePackageRevision(ctx, obj)
if err != nil {
return nil, err
Expand Down Expand Up @@ -367,107 +358,6 @@ func ensureUniqueWorkspaceName(obj *api.PackageRevision, existingRevs []reposito
return nil
}

func ensureSameOrigin(ctx context.Context, repo repository.Repository, obj *api.PackageRevision, existingRevs []repository.PackageRevision) (bool, error) {
if len(existingRevs) == 0 {
// no prior package revisions, no need to check anything else
return true, nil
}

tasks := obj.Spec.Tasks
if len(tasks) == 0 || !taskTypeOneOf(tasks[0].Type, api.TaskTypeInit, api.TaskTypeClone, api.TaskTypeEdit) {
// If there are no tasks, or the first task is not init or clone, then this revision was not
// created from another package revision. That means we expect it to be the first revision
// for this package.
return false, nil
}

firstObjTask := tasks[0].DeepCopy()

// Edit tasks is a copy of a different revision in the same package, so therefore must
// always be allowed.
if firstObjTask.Type == api.TaskTypeEdit {
return true, nil
}

// iterate over existing package revisions, and look for one with a matching init or clone task.
// We consider a packagerevision to have the same upstream if we find at least one existing
// packagerevision that matches.
for _, rev := range existingRevs {
p, err := rev.GetPackageRevision(ctx)
if err != nil {
return false, err
}
revTasks := p.Spec.Tasks
if len(revTasks) == 0 {
// not a match
continue
}
firstRevTask := revTasks[0].DeepCopy()
if firstRevTask.Type != firstObjTask.Type {
// not a match
continue
}

if firstObjTask.Type == api.TaskTypeClone {
// the user is allowed to update the git upstream ref, so we exclude that from the equality check.
// We make the git upstream refs equal before calling reflect.DeepEqual
if firstRevTask.Clone == nil || firstObjTask.Clone == nil {
continue
}

switch {
// If both the new and existing packagerevision has a git upstream, we consider it
// from the same upstream if the repo and directory are the same.
case firstRevTask.Clone.Upstream.Git != nil && firstObjTask.Clone.Upstream.Git != nil:
firstRevTask.Clone.Upstream.Git.Ref = firstObjTask.Clone.Upstream.Git.Ref
if reflect.DeepEqual(firstRevTask, firstObjTask) {
return true, nil
}
// If both the new and existing packagerevision has an oci upstream, we consider it
// from the same upstream if the image path is the same. We do not care about any tag
// or digest.
case firstRevTask.Clone.Upstream.Oci != nil && firstObjTask.Clone.Upstream.Oci != nil:
objOciImage := getBaseImage(firstObjTask.Clone.Upstream.Oci.Image)
revOciImage := getBaseImage(firstRevTask.Clone.Upstream.Oci.Image)
if objOciImage == revOciImage {
firstRevTask.Clone.Upstream.Oci.Image = firstObjTask.Clone.Upstream.Oci.Image
}
if reflect.DeepEqual(firstRevTask, firstObjTask) {
return true, nil
}
// If both the new and existing packagerevision has a porch upstream, we dereference the
// upstreamRef and make sure that both reference revisions of the same package.
case firstRevTask.Clone.Upstream.UpstreamRef != nil && firstObjTask.Clone.Upstream.UpstreamRef != nil:
revRepoPkgRev, found, err := getPackageRevision(ctx, repo, firstObjTask.Clone.Upstream.UpstreamRef.Name)
if err != nil {
return false, err
}
if !found {
continue
}

objRepoPkgRev, found, err := getPackageRevision(ctx, repo, firstRevTask.Clone.Upstream.UpstreamRef.Name)
if err != nil {
return false, err
}
if !found {
continue
}
if revRepoPkgRev.Key().Repository == objRepoPkgRev.Key().Repository &&
revRepoPkgRev.Key().Package == objRepoPkgRev.Key().Package {
return true, nil
}

}
} else {
if reflect.DeepEqual(firstRevTask, firstObjTask) {
return true, nil
}
}
}
return false, nil
}

func getPackageRevision(ctx context.Context, repo repository.Repository, name string) (repository.PackageRevision, bool, error) {
repoPkgRevs, err := repo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{
KubeObjectName: name,
Expand Down
Loading

0 comments on commit 5ad4116

Please sign in to comment.