Allow to add/switch components of a published local repository #1356

cfiehe · 2024-10-02T19:57:14Z

Description of the Change

This commit modifies the behavior of the publish switch method in the way, that it can also be used to add/update components of published local repositories. Furthermore, the commit ensures that the sources returned by a published repository are ordered by their component names.

Checklist

functional test added/updated (if change is functional)
man page updated (if applicable)
documentation updated
author name in AUTHORS
make sure slash can be used in prefix (Update Publish does not suport slash in prefix #1338)
swagger documentation

neolynx · 2024-10-02T21:19:43Z

changing the logging is a bit tricky, since the aptly output is used extensively in the testing. but I like the cleanup !

if you wanna update the tests with the new logs automatically, uncomment CAPTURE in the Makefile locally and run the system-tests (it will write new gold files) ;-)

cfiehe · 2024-10-02T21:31:10Z

Thanks a lot. I was not aware of the consequences. Unfortunately, we must change the output of the switch command and make it more generic, so that it can be also used for published local repositories. It requires some time for fixing the test cases.

neolynx · 2024-10-02T23:14:59Z

amazing work :)

I sent you an invitation as maintainer, it would be great to have more of your contribution as well as reviewing some PRs (i.e. #1352) to move things forward a bit faster !

neolynx · 2024-10-02T23:41:04Z

I dont have the rights to push to your branch, but here would be the golang-lint fix: eb15a65 :-)

neolynx · 2024-10-02T23:44:44Z

there is one more log update in a S3 test (where your i does not have creds): https://github.com/aptly-dev/aptly/actions/runs/11153050817/job/30999844378#step:9:1456

cfiehe · 2024-10-03T06:34:34Z

@neolynx
Have you got an idea, why the test execution is failing?

--- PASS: Test (0.01s)
PASS
coverage: 74.9% of statements
ok  	github.com/aptly-dev/aptly/utils	0.011s	coverage: 74.9% of statements
FAIL

Stopping etcd ...

Unit Tests FAILED

cfiehe · 2024-10-03T07:01:53Z

I dont have the rights to push to your branch, but here would be the golang-lint fix: eb15a65 :-)

Ok, that is strange. The flag that the maintainer has editing rights is set.

This commit modifies the behavior of the publish switch method in the way, that it can also be used to add/switch components of a published local repository. Furthermore, the commit ensures that the sources returned by a published repository are ordered by their component names. Signed-off-by: Christoph Fiehe <c.fiehe@eurodata.de>

neolynx · 2024-10-03T13:29:53Z

coverage 76.85% of diff hit (target 74.90%)

nice !

cfiehe · 2024-10-03T13:42:55Z

It finally turns into green 😅. Thanks a lot for your support.

neolynx · 2024-10-03T13:48:53Z

api/publish.go

 func apiPublishUpdateSwitch(c *gin.Context) {
 	param := parseEscapedPath(c.Params.ByName("prefix"))
 	storage, prefix := deb.ParsePrefix(param)
-	distribution := c.Params.ByName("distribution")
+	distribution := parseEscapedPath(c.Params.ByName("distribution"))

 	var b struct {


need to document the body json parameters: See https://github.com/aptly-dev/aptly/blob/feature/176-snapshot-pull-api/api/snapshot.go#L506

neolynx · 2024-10-03T13:50:32Z

api/publish.go

@@ -363,14 +452,27 @@ func apiPublishUpdateSwitch(c *gin.Context) {
 	})
 }

-// DELETE /publish/:prefix/:distribution
+// @Summary Delete published repository
+// @Description Delete a published repository.


make bold, no dot: **Delete a published repository**

copy api and field description from here: https://www.aptly.info/doc/api/publish/

neolynx · 2024-10-03T13:54:47Z

api/publish.go

+}
+
+// @Summary Create published repository
+// @Description Create a published repository with specified parameters.


document json params

make description bold, not dot

neolynx · 2024-10-03T14:01:27Z

Another more high level question: if we add a /remove in the other PR, shouldn't we add a /add here instead of modifying the existing update/switch ?

We could also keep the switch/update not touching the snapshots/components, but rather remember the multi-dist, and add 2 new APIs for adding/removing them, maybe call them /extend and /redude ?

just thinking loudly, now that I see the full picture ... :)

neolynx · 2024-10-03T14:23:23Z

api/publish.go

 func apiPublishUpdateSwitch(c *gin.Context) {
 	param := parseEscapedPath(c.Params.ByName("prefix"))
 	storage, prefix := deb.ParsePrefix(param)
-	distribution := c.Params.ByName("distribution")
+	distribution := parseEscapedPath(c.Params.ByName("distribution"))


cfiehe · 2024-10-03T14:37:21Z

Another more high level question: if we add a /remove in the other PR, shouldn't we add a /add here instead of modifying the existing update/switch ?

Yes, I had the same thoughts. It "feels" a little bit asymmetric, having a remove command in the CLI and in the REST API, but no corresponding add command.

We could also keep the switch/update not touching the snapshots/components, but rather remember the multi-dist, and add 2 new APIs for adding/removing them, maybe call them /extend and /redude ?

I am not sure, if I get that right. Before the enhancements, the switch/update in the REST API and switch in the CLI were used for switching snapshots of a published snapshot repository. So even before the change, these commands touched snapshots/components, but they were limited to published snapshot repositories and could not be used for published local repositories.

We can introduce two new commands in the CLI publish group, e.g. extend and reduce (typo above?) or add and remove what you like more. These commands can be used to add new components to a published repository or remove existing components from a published repository. But how to handle updates? Should we stick to the existing switch command for changing the sources (snapshot, local repo) for components that are already part of a published repository?

In the REST API, we can introduce extend and reduce or add and remove as new subresources of a published repository. But how to handle updates? The current approach is to use PUT on the published repository resource. Do you want to change that?

neolynx · 2024-10-03T16:14:23Z

api/publish.go

-// @Summary Get publish points
-// @Description Get list of available publish points. Each publish point is returned as in “show” API.
+// @Summary Get published repositories
+// @Description Get a list of published repositories. Each published repository is returned as in "show" API.


make bold with **, no dot

neolynx · 2024-10-03T16:15:31Z

api/publish.go

@@ -245,11 +289,22 @@ func apiPublishRepoOrSnapshot(c *gin.Context) {
 	})
 }

-// PUT /publish/:prefix/:distribution
+// @Summary Update published repository


copy multi line description from https://www.aptly.info/doc/api/publish/,

neolynx · 2024-10-03T16:25:14Z

I am not sure, if I get that right. Before the enhancements, the switch/update in the REST API and switch in the CLI were used for switching snapshots of a published snapshot repository. So even before the change, these commands touched snapshots/components, but they were limited to published snapshot repositories and could not be used for published local repositories.

ok, so the change here is that we also allow local repos. adding new snapshots was already possible in that since, right ?

add and remove what you like more

I think add/remove is better.

But how to handle updates? Should we stick to the existing switch command for changing the sources (snapshot, local repo) for components that are already part of a published repository?

what I fail to wrap my head around: with aptly commands we have publish update and publish switch. In the API both are done with the PUT ? What is the difference between switch and update ?

In the REST API, we can introduce extend and reduce or add and remove as new subresources of a published repository. But how to handle updates? The current approach is to use PUT on the published repository resource. Do you want to change that?

I would not change existing behavior, therefore just adding /remove is the least invasive. We can still add /add later.

Only the documentation needs to be updated (and hopefully bring some clarity about switch/update/remove). Maybe you can have a look at #1352 and #1162, if we merge them first and rebase your PRs, we have a good base for the pipeline and documentation...

cfiehe · 2024-10-03T17:42:57Z

ok, so the change here is that we also allow local repos. adding new snapshots was already possible in that since, right ?

Before we started, you had to define all components you want to have in your published repository at creation time. Afterwards, it was not possible to add or remove components from a published repository of any kind (snapshot or local). Adding or removing requires dropping the published repository and recreating it by which the new component set. That means the component set of a published repository was immutable and could not be modified after creation. The switch command allows you – only in case of a published snapshot repository – to replace a snapshot associated with a component. The update command tells the published repository – only in the case of a published local repository – to refresh the packages from the backed local repository. But it was not possible to associate an existing component with another local repository. In contrast to a published snapshot repository, the mapping between component and local repository was immutable and could not be changed after creation.

For example, we have defined a mirror of all packages in the main component of Ubuntu 24.04. Now, we create the snapshot ubuntu-noble-main-1 and publish it in a published snapshot repository by associating the main component with the snapshot main -> ubuntu-noble-main#1. The switch command allows you to associate another snapshot with the main component, e.g. main -> ubuntu-noble-main#2. Now you reach the point where you e.g. also want to have all packages in your published repository, which come from the universe component. You define a new mirror and create the snapshot ubuntu-noble-universe#1. The problem is, that the switch command can only operate on components that are already inside your published repository, you cannot use it to add the new universe component. The former approach was to drop your published repository and recreate with the sources main -> ubuntu-noble-main#2 and universe -> ubuntu-noble-universe#1. After some time, you want to get rid of the universe component again, because the packages may be no longer required. You can use the switch command to associate e.g. an empty snapshot with the universe component, but you cannot actually drop it from your published repository without recreating the published repository.

What we are currently trying to achieve is, that Aptly offers a way to modify the component set after creation time by adding and removing components and by associating them with snapshots or local repositories, respectively. The following use case were not supported before our changes:

Add components to a published repository of any kind
Remove components from a published snapshot of any kind
Associate a component with another local repository in case of a published local repository (the switch command already allowed that in case of published snapshot repository)

cfiehe · 2024-10-03T17:49:27Z

what I fail to wrap my head around: with aptly commands we have publish update and publish switch. In the API both are done with the PUT ? What is the difference between switch and update ?

A snapshot is an immutable package set. Calling update on a published snapshot repository will never pull in new packages and is therefore not supported by Aptly. If you want to change your package set, you must switch to another snapshot which may contain a different package set (publish switch).

However, in case of a local repository someone could have added or removed packages. In order to make them accessible from the outside, you must advice the corresponding published local repository to re-fetch the package set from the backed local repositories (publish update).

The two commands have in common that they influence the package set of a published repository. But there is no other functional relationship between them. switch operates on the component set and modifies the mapping between component and source, update does not do anything like that.

cfiehe · 2024-10-03T18:20:12Z

Summary:

Introduce publish add to add new components to a publish repository
Introduce publish remove to remove existing components from a publish repository
Use publish switch to change the mapping between a component and its package source (snapshot/local repository)

Maybe it is possible to add a new subcommand group to publish, but we loose backward compatibility for publish switch:

aptly publish source add
aptly publish source remove
aptly publish source update

cfiehe · 2024-10-03T18:21:27Z

That would be an alternative REST-API but it is not so RESTful, because we actually model actions and it has the problem, that switch is currently done by via PUT on /api/publish/{prefix}/{distribution}.

/api/publish/{prefix}/{distribution}/add [post]
/api/publish/{prefix}/{distribution}/switch [post]
/api/publish/{prefix}/{distribution}/remove [post]
/api/publish/{prefix}/{distribution}/update [post]

or

/api/publish/{prefix}/{distribution}/sources [post, get] - Single source creation, List sources
/api/publish/{prefix}/{distribution}/sources-bulk [post] - Multiple operations (Creates, Updates, Removes)
/api/publish/{prefix}/{distribution}/update [post]

We model /sources-bulk as bulk operation resource allowing to execute multiple operations on more than one source in a single request. It would be enough to only implement /sources-bulk.

Inspired by:

What do you think?

Maybe we can keep the old way to switch a snapshot of a published snapshot repository for some time by using PUT on /api/publish/{prefix}/{distribution} and give a deprecation warning, we could do the same for triggering a package update for a published local repository. We could add MultiDist and make it modifiable like SkipContents and Co.

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch 4 times, most recently from 0d96e4b to 18ffde2 Compare October 2, 2024 20:27

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch from 18ffde2 to bcab28d Compare October 2, 2024 21:21

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch 4 times, most recently from 9c64533 to eb6acfd Compare October 2, 2024 22:49

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch 2 times, most recently from 1924941 to e97725c Compare October 3, 2024 06:27

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch from e97725c to 744c0d9 Compare October 3, 2024 07:00

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch 7 times, most recently from 01a4452 to 1a9638b Compare October 3, 2024 13:03

cfiehe force-pushed the feature/allow_add_component_to_published_repo_for_local_repo branch from 1a9638b to 584146f Compare October 3, 2024 13:17

neolynx added the needs review Ready for review & merge label Oct 3, 2024

cfiehe requested a review from neolynx October 3, 2024 13:43

neolynx reviewed Oct 3, 2024

View reviewed changes

neolynx assigned neolynx and cfiehe and unassigned neolynx Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to add/switch components of a published local repository #1356

Allow to add/switch components of a published local repository #1356

cfiehe commented Oct 2, 2024 •

edited by neolynx

Loading

neolynx commented Oct 2, 2024 •

edited

Loading

cfiehe commented Oct 2, 2024

neolynx commented Oct 2, 2024

neolynx commented Oct 2, 2024

neolynx commented Oct 2, 2024

cfiehe commented Oct 3, 2024

cfiehe commented Oct 3, 2024

neolynx commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024

neolynx Oct 3, 2024

neolynx Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024

neolynx Oct 3, 2024

neolynx commented Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024

cfiehe commented Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024

neolynx Oct 3, 2024

neolynx commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024

cfiehe commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading

Allow to add/switch components of a published local repository #1356

Are you sure you want to change the base?

Allow to add/switch components of a published local repository #1356

Conversation

cfiehe commented Oct 2, 2024 • edited by neolynx Loading

Description of the Change

Checklist

neolynx commented Oct 2, 2024 • edited Loading

cfiehe commented Oct 2, 2024

neolynx commented Oct 2, 2024

neolynx commented Oct 2, 2024

neolynx commented Oct 2, 2024

cfiehe commented Oct 3, 2024

cfiehe commented Oct 3, 2024

neolynx commented Oct 3, 2024 • edited Loading

cfiehe commented Oct 3, 2024

neolynx Oct 3, 2024

Choose a reason for hiding this comment

neolynx Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

neolynx Oct 3, 2024

Choose a reason for hiding this comment

neolynx Oct 3, 2024

Choose a reason for hiding this comment

neolynx commented Oct 3, 2024 • edited Loading

neolynx Oct 3, 2024

Choose a reason for hiding this comment

cfiehe commented Oct 3, 2024 • edited Loading

neolynx Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

neolynx Oct 3, 2024

Choose a reason for hiding this comment

neolynx Oct 3, 2024

Choose a reason for hiding this comment

neolynx commented Oct 3, 2024 • edited Loading

cfiehe commented Oct 3, 2024

cfiehe commented Oct 3, 2024 • edited Loading

cfiehe commented Oct 3, 2024 • edited Loading

cfiehe commented Oct 3, 2024 • edited Loading

cfiehe commented Oct 2, 2024 •

edited by neolynx

Loading

neolynx commented Oct 2, 2024 •

edited

Loading

neolynx commented Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024 •

edited

Loading

neolynx commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading

neolynx Oct 3, 2024 •

edited

Loading

neolynx commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading

cfiehe commented Oct 3, 2024 •

edited

Loading