Skip to content

Commit 5644232

Browse files
kushnaidukushnaidu
authored
Fix upgrade packageRevision filtering (#416)
* Fix an issue where NewUpstreamRef is not filtering packages properly * fix upstream discovery to work with repos registered with directories
1 parent 3432bd5 commit 5644232

5 files changed

Lines changed: 272 additions & 11 deletions

File tree

‎pkg/cli/commands/rpkg/upgrade/command.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,18 @@ func (r *runner) doUpgrade(pr *porchapi.PackageRevision) (*porchapi.PackageRevis
183183
if !oldUpstreamPr.IsPublished() {
184184
return nil, pkgerrors.Errorf("old upstream package revision %s is not published", oldUpstreamPr.Name)
185185
}
186+
upstreamPackageName := oldUpstreamPr.Spec.PackageName
187+
upstreamRepoName := oldUpstreamPr.Spec.RepositoryName
186188
var newUpstreamPr *porchapi.PackageRevision
187189
if r.revision == 0 {
188-
newUpstreamPr = r.findLatestPackageRevisionForRef(oldUpstreamPr.Spec.PackageName)
190+
newUpstreamPr = r.findLatestPackageRevisionForRef(upstreamPackageName, upstreamRepoName)
189191
if newUpstreamPr == nil {
190-
return nil, pkgerrors.Errorf("failed to find latest published revision for package %s (--revision was %d)", pr.Spec.PackageName, r.revision)
192+
return nil, pkgerrors.Errorf("failed to find latest published revision for package %s in repo %s (--revision was %d)", upstreamPackageName, upstreamRepoName, r.revision)
191193
}
192194
} else {
193-
newUpstreamPr = r.findPackageRevisionForRef(oldUpstreamPr.Spec.PackageName, r.revision)
195+
newUpstreamPr = r.findPackageRevisionForRef(upstreamPackageName, upstreamRepoName, r.revision)
194196
if newUpstreamPr == nil {
195-
return nil, pkgerrors.Errorf("revision %d does not exist for package %s", r.revision, pr.Spec.PackageName)
197+
return nil, pkgerrors.Errorf("revision %d does not exist for package %s in repo %s", r.revision, upstreamPackageName, upstreamRepoName)
196198
}
197199
}
198200

@@ -249,21 +251,21 @@ func (r *runner) findPackageRevision(prName string) *porchapi.PackageRevision {
249251
return nil
250252
}
251253

252-
func (r *runner) findPackageRevisionForRef(name string, revision int) *porchapi.PackageRevision {
254+
func (r *runner) findPackageRevisionForRef(name, repo string, revision int) *porchapi.PackageRevision {
253255
for i := range r.prs {
254256
pr := r.prs[i]
255-
if pr.Spec.PackageName == name && pr.IsPublished() && pr.Spec.Revision == revision {
257+
if pr.Spec.PackageName == name && pr.Spec.RepositoryName == repo && pr.IsPublished() && pr.Spec.Revision == revision {
256258
return &pr
257259
}
258260
}
259261
return nil
260262
}
261263

262-
func (r *runner) findLatestPackageRevisionForRef(name string) *porchapi.PackageRevision {
264+
func (r *runner) findLatestPackageRevisionForRef(name, repo string) *porchapi.PackageRevision {
263265
latest := 0
264266
var output *porchapi.PackageRevision
265267
for _, pr := range r.prs {
266-
if pr.Spec.PackageName == name && pr.IsPublished() && pr.Spec.Revision > latest {
268+
if pr.Spec.PackageName == name && pr.Spec.RepositoryName == repo && pr.IsPublished() && pr.Spec.Revision > latest {
267269
latest = pr.Spec.Revision
268270
output = &pr
269271
}

‎pkg/cli/commands/rpkg/upgrade/command_test.go‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func TestFindLatestPR(t *testing.T) {
296296

297297
r := createRunner(context.Background(), fake.NewClientBuilder().Build(), prs, "ns", 0)
298298

299-
found := r.findLatestPackageRevisionForRef("orig")
299+
found := r.findLatestPackageRevisionForRef("orig", "repo")
300300
assert.Equal(t, "repo.orig.v2", found.Name)
301301
assert.Equal(t, 2, found.Spec.Revision)
302302
}
@@ -978,3 +978,55 @@ func TestFindUpstreamInUpgradeTask(t *testing.T) {
978978

979979
assert.Equal(t, "new-upstream-v2", result)
980980
}
981+
982+
func TestAvailableUpdatesWithDirectory(t *testing.T) {
983+
const ns = "ns"
984+
ctx := context.Background()
985+
986+
// Create upstream package revisions in a repo with directory configured
987+
upstreamPkg1 := createOrigPackageRevision(ns, "blueprints", "pkg", 1)
988+
upstreamPkg2 := createEditPackageRevision(upstreamPkg1, 2)
989+
upstreamPkg3 := createEditPackageRevision(upstreamPkg1, 3)
990+
991+
// Create downstream package that references upstream with directory in upstreamLock
992+
downstreamPkg := createClonePackageRevision(upstreamPkg1, "downstream-pkg", 1)
993+
downstreamPkg.Status.UpstreamLock = &porchapi.Locator{
994+
Git: &porchapi.GitLock{
995+
Repo: "https://github.com/user/repo.git",
996+
Ref: "packages/pkg/v1", // includes directory prefix
997+
},
998+
}
999+
1000+
prs := []porchapi.PackageRevision{*upstreamPkg1, *upstreamPkg2, *upstreamPkg3, *downstreamPkg}
1001+
1002+
// Create repository with directory configured
1003+
repoWithDir := configapi.Repository{
1004+
ObjectMeta: metav1.ObjectMeta{
1005+
Name: "blueprints",
1006+
Namespace: ns,
1007+
},
1008+
Spec: configapi.RepositorySpec{
1009+
Type: configapi.RepositoryTypeGit,
1010+
Git: &configapi.GitRepository{
1011+
Repo: "https://github.com/user/repo.git",
1012+
Directory: "/packages",
1013+
},
1014+
},
1015+
}
1016+
1017+
repoList := &configapi.RepositoryList{
1018+
Items: []configapi.Repository{repoWithDir},
1019+
}
1020+
1021+
r := createRunner(ctx, fake.NewClientBuilder().Build(), prs, ns, 0)
1022+
1023+
// Test that availableUpdates finds v2 and v3 when downstream is at v1
1024+
availableUpdates, upstreamName, draftName, err := r.availableUpdates(downstreamPkg.Status.UpstreamLock, repoList)
1025+
1026+
assert.NoError(t, err)
1027+
assert.Equal(t, "blueprints", upstreamName)
1028+
assert.Equal(t, "", draftName)
1029+
assert.Len(t, availableUpdates, 2, "Should find v2 and v3 as available updates")
1030+
assert.Equal(t, 2, availableUpdates[0].Spec.Revision)
1031+
assert.Equal(t, 3, availableUpdates[1].Spec.Revision)
1032+
}

‎pkg/cli/commands/rpkg/upgrade/discover.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,12 @@ func (r *runner) availableUpdates(upstreamLock *porchapi.Locator, repositories *
147147
}
148148
if upstreamLock.Git.Repo == repo.Spec.Git.Repo {
149149
upstream = repo.Name
150-
revisions = r.getUpstreamRevisions(repo, upstreamPackageName)
150+
// If the repo has a directory configured, strip it from the upstream package name
151+
pkgNameToMatch := upstreamPackageName
152+
if repo.Spec.Git != nil && repo.Spec.Git.Directory != "" {
153+
pkgNameToMatch = strings.TrimPrefix(upstreamPackageName, strings.TrimPrefix(repo.Spec.Git.Directory, "/")+"/")
154+
}
155+
revisions = r.getUpstreamRevisions(repo, pkgNameToMatch)
151156
}
152157
}
153158

@@ -175,7 +180,16 @@ func (r *runner) getUpstreamRevisions(repo configapi.Repository, upstreamPackage
175180
// only consider published packages
176181
continue
177182
}
178-
if pkgRev.Spec.RepositoryName == repo.Name && pkgRev.Spec.PackageName == upstreamPackageName {
183+
if pkgRev.Spec.RepositoryName != repo.Name {
184+
continue
185+
}
186+
// The package name in the repo might have a directory prefix if the repo has a directory configured
187+
pkgName := pkgRev.Spec.PackageName
188+
if repo.Spec.Git != nil && repo.Spec.Git.Directory != "" {
189+
// Strip the directory prefix if present
190+
pkgName = strings.TrimPrefix(pkgName, repo.Spec.Git.Directory+"/")
191+
}
192+
if pkgName == upstreamPackageName {
179193
result = append(result, pkgRev)
180194
}
181195
}

‎test/e2e/cli/suite.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,25 @@ func deleteRemoteTestRepo(t *testing.T, testcaseName string) {
420420
} else {
421421
t.Logf("Failed to delete repo: %s %s\n", testcaseName, resp.Status)
422422
}
423+
424+
if testcaseName == "rpkg-upgrade" {
425+
apiURL2 := fmt.Sprintf("http://localhost:3000/api/v1/repos/%s/rpkg-upgrade-downstream", testGitUserOrg)
426+
req2, err := nethttp.NewRequest("DELETE", apiURL2, nil)
427+
if err != nil {
428+
t.Fatalf("Failed to create DELETE request: %v", err)
429+
}
430+
req2.Header.Set("Authorization", basicAuth)
431+
resp2, err := nethttp.DefaultClient.Do(req2)
432+
if err != nil {
433+
t.Fatalf("Failed to make request: %v", err)
434+
}
435+
defer resp2.Body.Close()
436+
if resp2.StatusCode == nethttp.StatusNoContent {
437+
t.Logf("Repo deleted successfully: rpkg-upgrade-downstream")
438+
} else {
439+
t.Logf("Failed to delete repo: rpkg-upgrade-downstream %s\n", resp2.Status)
440+
}
441+
}
423442
}
424443

425444
func createRemoteTestRepo(t *testing.T, testcaseName string) {
@@ -479,6 +498,64 @@ func createRemoteTestRepo(t *testing.T, testcaseName string) {
479498
t.Fatalf("Failed to push test repo %s: %v", testcaseName, err)
480499
}
481500
t.Logf("Test repo created successfully: %s", testcaseName)
501+
502+
if testcaseName == "rpkg-upgrade" {
503+
tmpPath2 := t.TempDir()
504+
repo2, err := git.PlainInit(tmpPath2, false)
505+
if err != nil {
506+
t.Fatalf("Failed to init the repo rpkg-upgrade-downstream: %v", err)
507+
}
508+
509+
err = repo2.Storer.SetReference(
510+
plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.NewBranchReferenceName("main")),
511+
)
512+
if err != nil {
513+
t.Fatalf("Failed to set refs: %v", err)
514+
}
515+
516+
err = os.WriteFile(tmpPath2+"/README.md", []byte("# Test Go-Git Repo\nCreated programmatically."), 0644)
517+
if err != nil {
518+
t.Fatalf("Failed to write to file: %v", err)
519+
}
520+
521+
wt2, _ := repo2.Worktree()
522+
_, err = wt2.Add("README.md")
523+
if err != nil {
524+
t.Fatalf("Failed to add README: %v", err)
525+
}
526+
_, err = wt2.Commit("Initial commit", &git.CommitOptions{
527+
Author: &object.Signature{
528+
Name: "Nephio O' Test",
529+
Email: "nephiotest@example.com",
530+
When: time.Now(),
531+
},
532+
})
533+
if err != nil {
534+
t.Fatalf("Failed to commit to repo rpkg-upgrade-downstream: %v", err)
535+
}
536+
537+
repoUrl2 := fmt.Sprintf("http://localhost:3000/%s/rpkg-upgrade-downstream", testGitUserOrg)
538+
_, err = repo2.CreateRemote(&config.RemoteConfig{
539+
Name: "origin",
540+
URLs: []string{repoUrl2},
541+
})
542+
if err != nil {
543+
t.Fatalf("Failed to create remote: %v", err)
544+
}
545+
546+
err = repo2.Push(&git.PushOptions{
547+
RemoteName: "origin",
548+
Auth: &http.BasicAuth{
549+
Username: testGitUserOrg,
550+
Password: testGitPassword,
551+
},
552+
RequireRemoteRefs: []config.RefSpec{},
553+
})
554+
if err != nil {
555+
t.Fatalf("Failed to push test repo rpkg-upgrade-downstream: %v", err)
556+
}
557+
t.Logf("Test repo created successfully: rpkg-upgrade-downstream")
558+
}
482559
}
483560

484561
func getRepoName(args []string) (string, bool) {

‎test/e2e/cli/testdata/rpkg-upgrade/config.yaml‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,119 @@ commands:
186186
- git.upgrade-downstream.v1
187187
- --workspace=v2
188188
stdout: "git.upgrade-downstream.v1 upgraded to git.upgrade-downstream.v2\n"
189+
- args:
190+
- porchctl
191+
- repo
192+
- register
193+
- --namespace=rpkg-upgrade
194+
- --name=downstream1
195+
- --repo-basic-password=secret
196+
- --repo-basic-username=nephio
197+
- --deployment=true
198+
- --directory=downstream1
199+
- http://gitea.gitea.svc.cluster.local:3000/nephio/rpkg-upgrade-downstream
200+
- args:
201+
- porchctl
202+
- rpkg
203+
- clone
204+
- --namespace=rpkg-upgrade
205+
- git.upgrade-orig.v1
206+
- --repository=downstream1
207+
- --workspace=v1
208+
- upgrade-orig
209+
stdout: "downstream1.upgrade-orig.v1 created\n"
210+
- args:
211+
- porchctl
212+
- rpkg
213+
- propose
214+
- --namespace=rpkg-upgrade
215+
- downstream1.upgrade-orig.v1
216+
stdout: "downstream1.upgrade-orig.v1 proposed\n"
217+
- args:
218+
- porchctl
219+
- rpkg
220+
- approve
221+
- --namespace=rpkg-upgrade
222+
- downstream1.upgrade-orig.v1
223+
stdout: "downstream1.upgrade-orig.v1 approved\n"
224+
- args:
225+
- porchctl
226+
- rpkg
227+
- upgrade
228+
- --namespace=rpkg-upgrade
229+
- downstream1.upgrade-orig.v1
230+
- --workspace=v2
231+
stdout: "downstream1.upgrade-orig.v1 upgraded to downstream1.upgrade-orig.v2\n"
232+
- args:
233+
- kubectl
234+
- get
235+
- packagerevision
236+
- downstream1.upgrade-orig.v2
237+
- --namespace=rpkg-upgrade
238+
- -o=jsonpath={.spec.tasks[0].upgrade.newUpstreamRef.name}
239+
stdout: "git.upgrade-orig.v2\n"
240+
- args:
241+
- porchctl
242+
- rpkg
243+
- propose
244+
- --namespace=rpkg-upgrade
245+
- downstream1.upgrade-orig.v2
246+
stdout: "downstream1.upgrade-orig.v2 proposed\n"
247+
- args:
248+
- porchctl
249+
- rpkg
250+
- approve
251+
- --namespace=rpkg-upgrade
252+
- downstream1.upgrade-orig.v2
253+
stdout: "downstream1.upgrade-orig.v2 approved\n"
254+
- args:
255+
- porchctl
256+
- repo
257+
- register
258+
- --namespace=rpkg-upgrade
259+
- --name=downstream2
260+
- --repo-basic-password=secret
261+
- --repo-basic-username=nephio
262+
- --deployment=true
263+
- --directory=downstream2
264+
- http://gitea.gitea.svc.cluster.local:3000/nephio/rpkg-upgrade-downstream
265+
- args:
266+
- porchctl
267+
- rpkg
268+
- clone
269+
- --namespace=rpkg-upgrade
270+
- git.upgrade-orig.v1
271+
- --repository=downstream2
272+
- --workspace=v1
273+
- upgrade-orig
274+
stdout: "downstream2.upgrade-orig.v1 created\n"
275+
- args:
276+
- porchctl
277+
- rpkg
278+
- propose
279+
- --namespace=rpkg-upgrade
280+
- downstream2.upgrade-orig.v1
281+
stdout: "downstream2.upgrade-orig.v1 proposed\n"
282+
- args:
283+
- porchctl
284+
- rpkg
285+
- approve
286+
- --namespace=rpkg-upgrade
287+
- downstream2.upgrade-orig.v1
288+
stdout: "downstream2.upgrade-orig.v1 approved\n"
289+
- args:
290+
- porchctl
291+
- rpkg
292+
- upgrade
293+
- --namespace=rpkg-upgrade
294+
- downstream2.upgrade-orig.v1
295+
- --workspace=v2
296+
stdout: "downstream2.upgrade-orig.v1 upgraded to downstream2.upgrade-orig.v2\n"
297+
- args:
298+
- kubectl
299+
- get
300+
- packagerevision
301+
- downstream2.upgrade-orig.v2
302+
- --namespace=rpkg-upgrade
303+
- -o=jsonpath={.spec.tasks[0].upgrade.newUpstreamRef.name}
304+
stdout: "git.upgrade-orig.v2\n"

0 commit comments

Comments
 (0)