RPM repository content modify base-version expects the wrong type

[decko]

Summary

When using the following command:

pulp rpm repository content modify

the --base-version flag asks for an INTEGER instead of an URL, which is the correct.

Steps to reproduce

pulp rpm repository content modify --repository $REPO_NAME --base-version `$BASEVERSION_HREF`

should trigger an error message.
Also, you could use

pulp rpm repository content modify --help

and it gonna show that the --base-version flag expects an INTEGER instead of an URL.

Expected behavior

It's expected that --base-versions accepts an URL. Using an API call directly works flawlessly.

Stacktrace/Error log

Error: Invalid value for '--base-version': '/pulp/api/v3/repositories/rpm/rpm/8587fd87-494a-492e-a0af-ca0653e24a04/versions/1/' is not a valid integer.

Pulp and pulp-cli version info

pulp3 command line interface, version 0.17.0.dev0

{
  "versions": [
    {
      "component": "core",
      "version": "3.23.0.dev",
      "package": "pulpcore"
    },
    {
      "component": "rpm",
      "version": "3.19.0.dev",
      "package": "pulp-rpm"
    },
    {
      "component": "deb",
      "version": "2.21.0.dev",
      "package": "pulp_deb"
    },
    {
      "component": "container",
      "version": "2.15.0.dev",
      "package": "pulp-container"
    },
    {
      "component": "file",
      "version": "1.12.0.dev",
      "package": "pulp-file"
    }
  ],
  "online_workers": [
    {
      "pulp_href": "/pulp/api/v3/workers/750cbf26-8422-4827-8b9a-78306e9e7f4b/",
      "pulp_created": "2023-01-16T20:33:36.590792Z",
      "name": "569@9dcb6220b2d5",
      "last_heartbeat": "2023-01-16T20:56:10.028775Z",
      "current_task": null
    },
    {
      "pulp_href": "/pulp/api/v3/workers/9d3fa93a-f8ec-43df-839f-305dd64ab1d7/",
      "pulp_created": "2023-01-16T20:33:36.555093Z",
      "name": "525@9dcb6220b2d5",
      "last_heartbeat": "2023-01-16T20:56:10.032191Z",
      "current_task": null
    }
  ],
  "online_content_apps": [
    {
      "name": "578@9dcb6220b2d5",
      "last_heartbeat": "2023-01-16T20:56:10.735000Z"
    },
    {
      "name": "579@9dcb6220b2d5",
      "last_heartbeat": "2023-01-16T20:56:11.222660Z"
    }
  ],
  "database_connection": {
    "connected": true
  },
  "redis_connection": {
    "connected": false
  },
  "storage": {
    "total": 435432861696,
    "used": 56704614400,
    "free": 378728247296
  },
  "content_settings": {
    "content_origin": "http://localhost:5001",
    "content_path_prefix": "/pulp/content/"
  }
}

Additonal context

[mdellweg]

It does in fact ask for an integer, and that is by design:
https://github.com/pulp/pulp-cli/blob/4c392f03f7cef3372c063fc328a34b0cd6bc17af/pulpcore/cli/common
/generic.py#L759

How else would you refer to version 3 of a repository named "bar"?

[pedro-psb]

I dont see how base_version can be an integer with this design.

For example, I cannot say "copy version 2 of repository B into repository A".

In the REST API that would be:

http POST ":5001/${repoA_href}/modify/ base_version=${repoB_href}/versions/2/"

So in the cli it should be either:

# mirroing REST API 
pulp rpm repository content modify \
    --repository "${repoA_name}" \
    --base-version "${repoB_href}/versions/2/"  # informs which repo and which version

# assuming base-version is an int
pulp rpm repository content modify \
    --repository "${repoA_name}" \
    --base-repository "${repoB_name}" \  # which repo to copy from?
    --base-version "2"  # then an int is enough for the version

[mdellweg]

I don't think that the base version could be from a different repository.

[pedro-psb]

It can be from a different repository.
See how the base_version is passed to get_resources. It should be the uri for a RepositoryVersion, which is something like ${repo_href}/versions/N/

https://github.com/pulp/pulpcore/blob/a091ef251af7ad5f5342ebc76c52bff8e5738626/pulpcore/plugin/actions.py#L33

An example:

#!/bin/bash

# create and sync fixture repo
pulp rpm remote create --name fixtureRemote --policy on_demand --url "https://fixtures.pulpproject.org/rpm-signed/"
pulp rpm repository create --name fixtureRepository --remote fixtureRemote
pulp rpm repository sync --name fixtureRepository

# create empty repo
other_repo_href=$(pulp rpm repository create --name otherRepository | jq -r .pulp_href)

# copy from fixture to empty
fixture_repover_href=$(pulp rpm repository show --name fixtureRepository | jq -r .latest_version_href)
http POST :5001${other_repo_href}modify/ base_version=${fixture_repover_href}

# assert
pulp rpm repository content list --repository otherRepository | jq .[].location_href

[pedro-psb]

@mdellweg, wdyt? Maybe @ggainey can shime in too.

[ggainey]

@mdellweg, wdyt? Maybe @ggainey can shime in too.

I dug into the code, and it doesn't look like specifying a base-version-href that doesn't "belong to" the specified repository is "illegal" - which surprised me, like, A Lot. My expectation, and I think the general expectation, was that base-version is used when you want to modify a repo-version other than the latest in "this" repo. But that's not what the REST API enforces/requires.

So - is it that my expectation is Just Wrong? (which is very possible) In that case, we need to update docs with better examples, and the CLI needs to allow base-version-href (perhaps with "or integer, for version-in-'this'-repo) .

[pedro-psb]

Well, this is already two people expectations.

But my expectation was quite the opposite, because I related this feature ("modify:copy") as the way of, for example, copying a on-demand Content (and internally it's RA) from a different Repository, making it impossible to say a RA belongs to a Repository (probably there are other reasons we can't say that, but that's one of them).

And eventually similar cases pops up where we wanted to be able to say something about some entity and repositories, but we can't because we can copy stuff over (and that's an important thing people do).

---

If we agree on re-opening this, I would advocate for having base-repo: repo-name | repo-href and base-version: int. In case only base-version is provided, the base-repo defaults to the current repo we are modifying. If only base-repo is provided, the base-version defaults to the latest.

[ggainey]

When I the word "copy" I mean the Copy REST API (which is in RPM/Deb/Ansible). That's (for example) https://pulpproject.org/pulp_rpm/restapi/#tag/Rpm:-Copy, and https://pulpproject.org/pulp_rpm/docs/user/guides/modify/#full-repository-copy That API is very much intended to copy "all the content" from one repo to another, and absolutely has the "RemoteArtifact came from where?!?" problem.

[pedro-psb]

Discussion from the feature inception https://pulp.plan.io/issues/3360

But as of today, do we need these two endpoints for performing copy?
If not, we should warn-deprecate the modify:copy feature, or otherwise clarify what is it's use case compared to the copy. Maybe it is because not all plugins have the dedicated copy endpoint and inherit this for free?

[mdellweg]

Discussion from the feature inception https://pulp.plan.io/issues/3360

Given that old discussion, we can at least reopen the issue here.

[ggainey]

Given that old discussion, we can at least reopen the issue here.

Well. Given the commentary in the originating issue, we (or at the very least I!) am Just Wrong about the expectations on modify's base-version. We pretty clearly need to allow base-version-href, and take an int as a 'convenience' for "find the version-href for this repo with version-N"

[mdellweg]

Well we should accept (href | [[[plugin]:type]:name]:version]) in that case.