Fixes #39520 - Remove deprecated compute_resource endpoints by nadjaheitmann · Pull Request #11090 · theforeman/foreman

nadjaheitmann · 2026-07-07T11:33:16Z

I stumbled across that typo and actually was wondering if it is not time to remove that whole depreciation warning altogether :)

I don't see the parameter in there and the warning has been around since 2019!

nadjaheitmann · 2026-07-10T07:50:04Z

Thanks @chris1984 for approving. Any thoughts about removing the deprecation altogether? Maybe @ofedoren or @adamruzicka ?

ofedoren

Thanks, @nadjaheitmann :) There are even more typos :/

Well, if it was there for that amount of time and it's about dropping just a parameter instead of a whole endpoint, I personally don't mind :) Although, the deprecation warning itself was not quite correct, so 🤷

Also, if we're going to remove that, I'd like to ensure we're covered in other related places, such as hammer (does it use the suggested endpoint or still requires the param to be present) and maybe foreman ansible modules since they are built on top of API as well.

And a small OCD nitpick: please don't use Refs on something ancient :D It's better to create a separate ticket, especially if we're going to drop the deprecated stuff.

ofedoren · 2026-07-10T12:35:20Z

      def available_storage_domains
        if params[:storage_domain]
-          Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domain/:storage_domain_id endpoind instead")
+          Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domain/:storage_domain_id endpoint instead")


Suggested change

Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domain/:storage_domain_id endpoint instead")

Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domains/:storage_domain_id endpoint instead")

ofedoren · 2026-07-10T12:36:51Z

      def available_storage_pods
        if params[:storage_pod]
-          Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_pod/:storage_pod_id endpoind instead")
+          Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_pod/:storage_pod_id endpoint instead")


Suggested change

Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_pod/:storage_pod_id endpoint instead")

Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_pods/:storage_pod_id endpoint instead")

nadjaheitmann · 2026-07-10T16:48:36Z

Thanks, @nadjaheitmann :) There are even more typos :/

Thanks for finding them 🙈

Also, if we're going to remove that, I'd like to ensure we're covered in other related places, such as hammer (does it use the suggested endpoint or still requires the param to be present) and maybe foreman ansible modules since they are built on top of API as well.

I agree.

And a small OCD nitpick: please don't use Refs on something ancient :D It's better to create a separate ticket, especially if we're going to drop the deprecated stuff.

Personally, I think it looks weird but ok for the typo fix. If we want to drop the endpoint, we should for sure create a new ticket 👍

nadjaheitmann · 2026-07-14T08:44:09Z

@ofedoren @adamruzicka So what do you suggest to do? Merge the typo fix with @ofedoren s suggestion or go ahead and directly remove the deprecated parameter. I have opened an issue for it: https://projects.theforeman.org/issues/39520

adamruzicka · 2026-07-14T08:57:53Z

The deprecation warning was there for ~7 years and we have a major version coming up, I'd be tempted to drop it completely.

nadjaheitmann · 2026-07-14T09:26:46Z

The deprecation warning was there for ~7 years and we have a major version coming up, I'd be tempted to drop it completely.

Same here. Let me adjust the PR then.

ofedoren

Thanks, @nadjaheitmann, but here's your open can of worms :D

Since it's full removal, should we also remove the routes?

Also, I'm afraid there is a bit problem with the hammer counterpart: there is no command covering the suggested endpoints, so by removing the deprecated stuff here, we can't use alternative within hammer :/ Looking at the how it was written, we probably won't be able to make the changes "silently": we would either need to add a new specific command or maybe try to preserve the option, but use the new endpoint if it's used (an internal mess, but shouldn't break any automation).

No idea about FAM part though.

And it'll probably require a release note.

nadjaheitmann · 2026-07-17T06:54:16Z

Thanks @ofedoren !

Thanks, @nadjaheitmann, but here's your open can of worms :D

Well, I don't like worms, so let's clean them up :D

Since it's full removal, should we also remove the routes?

Yes, I adjusted the files.

Also, I'm afraid there is a bit problem with the hammer counterpart: there is no command covering the suggested endpoints, so by removing the deprecated stuff here, we can't use alternative within hammer :/ Looking at the how it was written, we probably won't be able to make the changes "silently": we would either need to add a new specific command or maybe try to preserve the option, but use the new endpoint if it's used (an internal mess, but shouldn't break any automation).

I agree that would be messy, but it's a good idea to preserve the hammer endpoint I guess.

No idea about FAM part though.

From what I can see, it might not affect FAM (https://docs.ansible.com/projects/ansible/latest/collections/theforeman/foreman/compute_resource_module.html#ansible-collections-theforeman-foreman-compute-resource-module), but I am no expert here, either, so please someone double-check. Maybe @evgeni ? Do you mind checking if removing this endpoint has an impact on FAM?

And it'll probably require a release note.

Yes, for sure. If we want to be nice, we can even announce it before the release such than community can prepare (I know that they had 7 years to prepare, but.... )

evgeni · 2026-07-17T07:04:00Z

Nice worms you have here, would be a shame if I offered you more, huh?

Just kiddin', we do use the /compute_resources/:id/available_storage_(domains|pods) and /compute_resources/:id/available_clusters/:cluster_id/available_storage_(domains|pods) endspoints, but not the two you're removing.

Burn them with fire! And glitter!

Those endpoints were deprecated in theforeman#6442 (which was in January 2019).

nadjaheitmann · 2026-07-17T10:03:31Z

Also, I'm afraid there is a bit problem with the hammer counterpart: there is no command covering the suggested endpoints, so by removing the deprecated stuff here, we can't use alternative within hammer :/ Looking at the how it was written, we probably won't be able to make the changes "silently": we would either need to add a new specific command or maybe try to preserve the option, but use the new endpoint if it's used (an internal mess, but shouldn't break any automation).

Just had a look at hammer. Which option are we talking about exactly? I found those two:

hammer -d compute-resource storage-pods --id 1 --storage-pod 1
calls
[ INFO 2026-07-17T09:43:58 API] GET /api/compute_resources/1/available_storage_pods/1 [DEBUG 2026-07-17T09:43:58 API] Params: {} [DEBUG 2026-07-17T09:43:58 API] Headers: { :params => {} }
and

hammer -d compute-resource storage-domain --id 1 --storage-domain 1

[ INFO 2026-07-17T10:01:17 API] GET /api/compute_resources/1/available_storage_domains/1 [DEBUG 2026-07-17T10:01:17 API] Params: {} [DEBUG 2026-07-17T10:01:17 API] Headers: { :params => {} }

ofedoren · 2026-07-17T13:48:25Z

Which option are we talking about exactly?

Those that are being removed: --storage-pod and --storage-domain. Those should be generated automatically based on the API docs. Then, if a user provided them, hammer should construct and call the endpoint we're removing (exactly what you have here: /api/compute_resources/1/available_storage_pods/1). The first 1 is the :id from the route, the second 1 is the :storage_pod from the route.

By removing those, hammer should no longer have these options, thus can't construct the call to the removed endpoint (which is expected). But since there are no longer options nor endpoint, hammer lost some functionality (even if it was weird to use like this) and has no replacement (yet). Given that I have no idea about actual usage of this endpoint through hammer, I'd be paranoid with the removal until there is an alternative present in hammer (or at least those options being added explicitly instead of being generated from API docs and pointed to construct the new route).

Regarding on how to approach this: no strong opinion. Surely it would be nice to just change internals, so the users wouldn't feel a thing, but on the other hand we could provide something new and be explicit about that in release notes and/or upgrade warnings.

nadjaheitmann · 2026-07-19T18:19:42Z

I had a short look but I could not figure out how to preserve the option for storage_pod and change the API action that is called in the background for only that one option. I can have another look once I am back from PTO.

pr-processor Bot added the Not yet reviewed label Jul 7, 2026

chris1984 approved these changes Jul 9, 2026

View reviewed changes

pr-processor Bot removed the Not yet reviewed label Jul 9, 2026

ofedoren reviewed Jul 10, 2026

View reviewed changes

nadjaheitmann force-pushed the test_rubocop branch 3 times, most recently from f4cc710 to 1184e4f Compare July 14, 2026 14:05

pr-processor Bot added the Waiting on contributor label Jul 14, 2026

nadjaheitmann force-pushed the test_rubocop branch from 1184e4f to 63f3932 Compare July 14, 2026 14:11

pr-processor Bot added Needs re-review and removed Waiting on contributor labels Jul 14, 2026

nadjaheitmann changed the title ~~Refs #2113 - Fix typo~~ Fixes #39520 - Remove deprecated compute_resource endpoints Jul 14, 2026

ofedoren reviewed Jul 15, 2026

View reviewed changes

pr-processor Bot removed the Needs re-review label Jul 15, 2026

nadjaheitmann force-pushed the test_rubocop branch from 63f3932 to 41335ad Compare July 17, 2026 06:47

evgeni reviewed Jul 17, 2026

View reviewed changes

Comment thread config/routes/api/v2.rb Outdated

evgeni reviewed Jul 17, 2026

View reviewed changes

Comment thread config/routes/api/v2.rb Outdated

Fixes #39520 - Remove deprecated compute_resource endpoints

fa59eca

Those endpoints were deprecated in theforeman#6442 (which was in January 2019).

nadjaheitmann force-pushed the test_rubocop branch from 41335ad to fa59eca Compare July 17, 2026 08:27

	Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domain/:storage_domain_id endpoint instead")
	Foreman::Deprecation.api_deprecation_warning("use /compute_resources/:id/storage_domains/:storage_domain_id endpoint instead")

Uh oh!

Conversation

nadjaheitmann commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nadjaheitmann commented Jul 10, 2026

Uh oh!

ofedoren left a comment

Choose a reason for hiding this comment

Uh oh!

ofedoren Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

ofedoren Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

nadjaheitmann commented Jul 10, 2026

Uh oh!

nadjaheitmann commented Jul 14, 2026

Uh oh!

adamruzicka commented Jul 14, 2026

Uh oh!

nadjaheitmann commented Jul 14, 2026

Uh oh!

ofedoren left a comment

Choose a reason for hiding this comment

Uh oh!

nadjaheitmann commented Jul 17, 2026

Uh oh!

evgeni commented Jul 17, 2026

Uh oh!

Uh oh!

Uh oh!

nadjaheitmann commented Jul 17, 2026

Uh oh!

ofedoren commented Jul 17, 2026

Uh oh!

nadjaheitmann commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nadjaheitmann commented Jul 7, 2026 •

edited

Loading