Skip to content

Commit 3705750

Browse files
authored
Compute Upstream Lock Ref Correctly (#3073)
1 parent 7865b26 commit 3705750

File tree

4 files changed

+143
-8
lines changed

4 files changed

+143
-8
lines changed

e2e/testdata/porch/rpkg-clone/config.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,14 @@ commands:
109109
upstream:
110110
git:
111111
directory: empty
112-
ref: v1
112+
ref: empty/v1
113113
repo: https://github.com/platkrm/test-blueprints.git
114114
type: git
115115
upstreamLock:
116116
git:
117117
commit: 3de8635354eda8e7de756494a4e0eb5c12af01ab
118118
directory: empty
119-
ref: v1
119+
ref: empty/v1
120120
repo: https://github.com/platkrm/test-blueprints.git
121121
type: git
122122
kind: ResourceList

porch/apiserver/pkg/e2e/e2e_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func (t *PorchSuite) TestCloneFromUpstream(ctx context.Context) {
217217
Git: &kptfilev1.GitLock{
218218
Repo: testBlueprintsRepo,
219219
Directory: "basens",
220-
Ref: "v1",
220+
Ref: "basens/v1",
221221
},
222222
}
223223
if !cmp.Equal(want, got) {
@@ -230,7 +230,7 @@ func (t *PorchSuite) TestCloneFromUpstream(ctx context.Context) {
230230
Git: &kptfilev1.Git{
231231
Repo: testBlueprintsRepo,
232232
Directory: "basens",
233-
Ref: "v1",
233+
Ref: "basens/v1",
234234
},
235235
}); !cmp.Equal(want, got) {
236236
t.Errorf("unexpected upstream returned (-want, +got) %s", cmp.Diff(want, got))
@@ -423,7 +423,7 @@ func (t *PorchSuite) TestCloneIntoDeploymentRepository(ctx context.Context) {
423423
Git: &kptfilev1.GitLock{
424424
Repo: testBlueprintsRepo,
425425
Directory: "basens",
426-
Ref: "v1",
426+
Ref: "basens/v1",
427427
},
428428
}
429429
if !cmp.Equal(want, got) {
@@ -436,7 +436,7 @@ func (t *PorchSuite) TestCloneIntoDeploymentRepository(ctx context.Context) {
436436
Git: &kptfilev1.Git{
437437
Repo: testBlueprintsRepo,
438438
Directory: "basens",
439-
Ref: "v1",
439+
Ref: "basens/v1",
440440
},
441441
}); !cmp.Equal(want, got) {
442442
t.Errorf("unexpected upstream returned (-want, +got) %s", cmp.Diff(want, got))

porch/repository/pkg/git/package.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,19 +157,28 @@ func (p *gitPackageRevision) GetUpstreamLock() (kptfile.Upstream, kptfile.Upstre
157157
return kptfile.Upstream{}, kptfile.UpstreamLock{}, fmt.Errorf("cannot determine package lock: %w", err)
158158
}
159159

160+
if p.ref == nil {
161+
return kptfile.Upstream{}, kptfile.UpstreamLock{}, fmt.Errorf("cannot determine package lock; package has no ref")
162+
}
163+
164+
ref, err := refInRemoteFromRefInLocal(p.ref.Name())
165+
if err != nil {
166+
return kptfile.Upstream{}, kptfile.UpstreamLock{}, fmt.Errorf("cannot determine package lock for %q: %v", p.ref, err)
167+
}
168+
160169
return kptfile.Upstream{
161170
Type: kptfile.GitOrigin,
162171
Git: &kptfile.Git{
163172
Repo: repo,
164173
Directory: p.path,
165-
Ref: p.revision,
174+
Ref: ref.Short(),
166175
},
167176
}, kptfile.UpstreamLock{
168177
Type: kptfile.GitOrigin,
169178
Git: &kptfile.GitLock{
170179
Repo: repo,
171180
Directory: p.path,
172-
Ref: p.revision,
181+
Ref: ref.Short(),
173182
Commit: p.commit.String(),
174183
},
175184
}, nil
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package git
16+
17+
import (
18+
"context"
19+
"path/filepath"
20+
"testing"
21+
22+
v1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
23+
"github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
24+
configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1"
25+
"github.com/GoogleContainerTools/kpt/porch/repository/pkg/repository"
26+
"github.com/go-git/go-git/v5/plumbing"
27+
"github.com/google/go-cmp/cmp"
28+
)
29+
30+
func (g GitSuite) TestUpstreamLock(t *testing.T) {
31+
tempdir := t.TempDir()
32+
tarfile := filepath.Join("testdata", "drafts-repository.tar")
33+
repo, address := ServeGitRepositoryWithBranch(t, tarfile, tempdir, g.branch)
34+
35+
ctx := context.Background()
36+
const (
37+
repositoryName = "lock"
38+
namespace = "default"
39+
)
40+
41+
git, err := OpenRepository(ctx, repositoryName, namespace, &configapi.GitRepository{
42+
Repo: address,
43+
Branch: g.branch,
44+
Directory: "/",
45+
}, tempdir, GitRepositoryOptions{})
46+
if err != nil {
47+
t.Fatalf("Failed to open Git repository loaded from %q: %v", tarfile, err)
48+
}
49+
50+
revisions, err := git.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{})
51+
if err != nil {
52+
t.Fatalf("Failed to list packages from %q: %v", tarfile, err)
53+
}
54+
55+
wantRefs := map[repository.PackageRevisionKey]string{
56+
{Repository: repositoryName, Package: "empty", Revision: "v1"}: "empty/v1",
57+
{Repository: repositoryName, Package: "basens", Revision: "v1"}: "basens/v1",
58+
{Repository: repositoryName, Package: "basens", Revision: "v2"}: "basens/v2",
59+
{Repository: repositoryName, Package: "istions", Revision: "v1"}: "istions/v1",
60+
{Repository: repositoryName, Package: "istions", Revision: "v2"}: "istions/v2",
61+
62+
{Repository: repositoryName, Package: "basens", Revision: g.branch}: g.branch,
63+
{Repository: repositoryName, Package: "empty", Revision: g.branch}: g.branch,
64+
{Repository: repositoryName, Package: "istions", Revision: g.branch}: g.branch,
65+
}
66+
67+
for _, rev := range revisions {
68+
if rev.Lifecycle() != v1alpha1.PackageRevisionLifecyclePublished {
69+
continue
70+
}
71+
72+
upstream, lock, err := rev.GetUpstreamLock()
73+
if err != nil {
74+
t.Errorf("GetUpstreamLock(%q) failed: %v", rev.Key(), err)
75+
}
76+
if got, want := upstream.Type, v1.GitOrigin; got != want {
77+
t.Errorf("upstream.Type: got %s, want %s", got, want)
78+
}
79+
if got, want := lock.Type, v1.GitOrigin; got != want {
80+
t.Errorf("lock.Type: got %s, want %s", got, want)
81+
}
82+
83+
key := rev.Key()
84+
wantRef, ok := wantRefs[key]
85+
if !ok {
86+
t.Errorf("Unexpected package found; %q", rev.Key())
87+
}
88+
89+
type gitAddress struct {
90+
Repo, Directory, Ref string
91+
}
92+
93+
// Check upstream values
94+
if got, want := (gitAddress{
95+
Repo: upstream.Git.Repo,
96+
Directory: upstream.Git.Directory,
97+
Ref: upstream.Git.Ref,
98+
}), (gitAddress{
99+
Repo: address,
100+
Directory: key.Package,
101+
Ref: wantRef,
102+
}); !cmp.Equal(want, got) {
103+
t.Errorf("Package upstream differs (-want,+got): %s", cmp.Diff(want, got))
104+
}
105+
106+
// Check upstream lock values
107+
if got, want := (gitAddress{
108+
Repo: lock.Git.Repo,
109+
Directory: lock.Git.Directory,
110+
Ref: lock.Git.Ref,
111+
}), (gitAddress{
112+
Repo: address,
113+
Directory: key.Package,
114+
Ref: wantRef,
115+
}); !cmp.Equal(want, got) {
116+
t.Errorf("Package upstream lock differs (-want,+got): %s", cmp.Diff(want, got))
117+
}
118+
119+
// Check the commit
120+
if commit, err := repo.ResolveRevision(plumbing.Revision(wantRef)); err != nil {
121+
t.Errorf("ResolveRevision(%q) failed: %v", wantRef, err)
122+
} else if got, want := lock.Git.Commit, commit.String(); got != want {
123+
t.Errorf("Commit: got %s, want %s", got, want)
124+
}
125+
}
126+
}

0 commit comments

Comments
 (0)