API changes breaking backward compatibility

Jeff · November 17, 2015, 10:27am

I've noticed things like this occasionally, especially before the v2 API
was the default but now its starting to cause issues.

I'm just starting to work on migrating from foreman 1.8 to the latest 1.10
RC2 and have some issue with the API changes…

Environments have changed. In foreman 1.8 the following API would return
details of the production environment…

/api/environments/production/

In 1.10rc2 this now gives an error:

"message": "Environment not found by id 'production'"

Now the API call must have the environment id rather than the name. Its not
a massive problem but it also affects related APIs (eg
/api/environments/production/puppetclasses). Since the environment name is
unique I dont see why this should not work. Conversely, the name-based
query of the hosts API works fine (/api/hosts/myhost.fqdn/)

This is the biggest problem I have come across so far. The structure of
the object returned by the foreman function (defined in
/usr/share/foreman-installer/modules/foreman/lib/puppet/parser/functions/foreman.rb)
has changed between versions. Example:

Query:
foreman({ item => 'environments'})

Object returned by foreman 1.8:
[{"environment"=>
{"id"=>2,
"name"=>"development",
"updated_at"=>"2015-10-28T14:09:25Z",
"created_at"=>"2015-10-28T14:09:25Z"}},
{"environment"=>
{"id"=>1,
"name"=>"production",
"updated_at"=>"2015-10-28T14:08:33Z",
"created_at"=>"2015-10-28T14:08:33Z"}}]

Object returned by foreman 1.10rc2:
{"sort"=>{"by"=>nil, "order"=>nil},
"subtotal"=>2,
"page"=>1,
"total"=>2,
"results"=>
[{"updated_at"=>"2015-11-16T15:53:38Z",
"created_at"=>"2015-11-16T15:53:38Z",
"id"=>2,
"name"=>"development"},
{"updated_at"=>"2015-11-16T15:34:07Z",
"created_at"=>"2015-11-16T15:34:07Z",
"id"=>1,
"name"=>"production"}],
"per_page"=>20,
"search"=>""}

The foreman function hasnt changed between versions, I guess its the data
being returned by the API. This change breaks any manifests using the data
returned by the foreman function.

Is this a bug or a "feature"?

Thanks
Jeff

Jeff · November 17, 2015, 11:21am

Perfect. Thanks!

Dominic_Cleal2 · November 17, 2015, 10:40am

> I've noticed things like this occasionally, especially before the v2 API
> was the default but now its starting to cause issues.
>
> I'm just starting to work on migrating from foreman 1.8 to the latest
> 1.10 RC2 and have some issue with the API changes…

Thanks for the reports, we do try and minimise any backwards
incompatible changes within an API version.

> 1) Environments have changed. In foreman 1.8 the following API would
> return details of the production environment…
>
> /api/environments/production/
>
> In 1.10rc2 this now gives an error:
>
>
> "message": "Environment not found by id 'production'"
>
>
> Now the API call must have the environment id rather than the name. Its
> not a massive problem but it also affects related APIs (eg
> /api/environments/production/puppetclasses). Since the environment name
> is unique I dont see why this should not work. Conversely, the
> name-based query of the hosts API works fine (/api/hosts/myhost.fqdn/)

This was a bug (Bug #12004: API: cannot query environments by name - Foreman), the fix is
in 1.9.3 and queued for 1.10.0-RC3 (today or tomorrow).

> 2) This is the biggest problem I have come across so far. The structure
> of the object returned by the foreman function (defined in
> /usr/share/foreman-installer/modules/foreman/lib/puppet/parser/functions/foreman.rb)
> has changed between versions. Example:
>
> Query:
> foreman({ item => 'environments'})
>
> Object returned by foreman 1.8:
> [{"environment"=>
> {"id"=>2,
> "name"=>"development",
> "updated_at"=>"2015-10-28T14:09:25Z",
> "created_at"=>"2015-10-28T14:09:25Z"}},
> {"environment"=>
> {"id"=>1,
> "name"=>"production",
> "updated_at"=>"2015-10-28T14:08:33Z",
> "created_at"=>"2015-10-28T14:08:33Z"}}]
>
> Object returned by foreman 1.10rc2:
> {"sort"=>{"by"=>nil, "order"=>nil},
> "subtotal"=>2,
> "page"=>1,
> "total"=>2,
> "results"=>
> [{"updated_at"=>"2015-11-16T15:53:38Z",
> "created_at"=>"2015-11-16T15:53:38Z",
> "id"=>2,
> "name"=>"development"},
> {"updated_at"=>"2015-11-16T15:34:07Z",
> "created_at"=>"2015-11-16T15:34:07Z",
> "id"=>1,
> "name"=>"production"}],
> "per_page"=>20,
> "search"=>""}
>
> The foreman function hasnt changed between versions, I guess its the
> data being returned by the API. This change breaks any manifests using
> the data returned by the foreman function.

This was done back in Foreman 1.9
(Foreman :: Manual), the
default unversioned access to the changed from version 1 to 2.

The foreman() function probably needs updating to handle version 2
properly and to request the API version it supports explicitly. In the
meantime you could change the URL it uses to /api/v1/ rather than plain
/api/.

···

On 17/11/15 10:27, Jeff wrote:

–
Dominic Cleal
dominic@cleal.org

Jeff · November 18, 2015, 1:46pm

rc3 has fixed the issue with the environment name. Thanks for that

Now another API problem - there doesnt seem to be a DELETE call on the
smart_class_parameter object. This was there in 1.8 but appears gone from
1.10 (and the apidocs). Was this a mistake?

DELETE /api/smart_class_parameters/329

ERROR: "HTTP Error 404: Not Found" when DELETE'ing
"/api/smart_class_parameters/329"
<!DOCTYPE html>
<html>
<head>
<title>The page you were looking for doesn't exist (404)</title>

Dominic_Cleal2 · November 18, 2015, 1:57pm

That's deliberate to match the UI, see "Disallow removal of smart class
parameters by a user" in the 1.10 release notes
(Foreman :: Manual).

The intention, like the earlier removal of the deletion of Puppet
classes is for it to work by import only.

···

On 18/11/15 13:46, Jeff wrote: > rc3 has fixed the issue with the environment name. Thanks for that :) > > Now another API problem - there doesnt seem to be a DELETE call on the > smart_class_parameter object. This was there in 1.8 but appears gone > from 1.10 (and the apidocs). Was this a mistake? > > DELETE /api/smart_class_parameters/329 > > ERROR: "HTTP Error 404: Not Found" when DELETE'ing > "/api/smart_class_parameters/329" > > > > The page you were looking for doesn't exist (404)

–
Dominic Cleal
dominic@cleal.org

Jeff · November 18, 2015, 2:18pm

Ouch, this causes a huge problem for us. Because of issue #2369 we have a
workaround using the API which basically deletes the sc parameter (from all
environments, since they appear to be global entities), then re-adds them
in the relevant environments by calling 'import puppetclasses'. Without the
delete method we cannot perform this workaround and puppet will fail on
run.

Are there plans to fix #2369?

···

On Wednesday, 18 November 2015 13:57:12 UTC, Dominic Cleal wrote: > > That's deliberate to match the UI, see "Disallow removal of smart class > parameters by a user" in the 1.10 release notes > (http://theforeman.org/manuals/1.10/index.html#Releasenotesfor1.10.0). > > The intention, like the earlier removal of the deletion of Puppet > classes is for it to work by import only. >

Konstantin_Orekhov · November 18, 2015, 5:47pm

One more thing I noticed in 1.10-rc - even though APIv2 is a default one
these days, discovered_hosts does not work unless "v2" is explicitly
specified:

[root@centos7 ~]# curl -kSs -u admin:<pass> https://localhost/api/v2/discovered_hosts
> jq '.total'
2

[root@centos7 ~]# curl -kSs -u admin:<pass>
https://localhost/api/discovered_hosts | jq '.total'
parse error: Invalid numeric literal at line 1, column 10

[root@centos7 ~]# curl -kSs -u admin:<pass>
https://localhost/api/discovered_hosts
<!DOCTYPE html>
<html>
<head>
<title>The page you were looking for doesn't exist (404)</title>
<style type="text/css">
body { background-color: #fff; color: #666; text-align: center;
font-family: arial, sans-serif; }
div.dialog {
width: 25em;
padding: 0 4em;
margin: 4em auto 0 auto;
border: 1px solid #ccc;
border-right-color: #999;
border-bottom-color: #999;
}
h1 { font-size: 100%; color: #f00; line-height: 1.5em; }
</style>
</head>

<body>

<div class="dialog">
<h1>The page you were looking for doesn't exist.</h1>
<p>You may have mistyped the address or the page may have moved.</p>
</div>
</body>
</html>
[root@centos7 ~]#

Dominic_Cleal2 · November 19, 2015, 8:00am

Could you file this as a bug against Discovery please? I think it might
need a change to the plugin's routing configuration to state that API
v2's the default when there's no version. (The plugin also ships no v1
API.)

···

On 18/11/15 17:47, 'Konstantin Orekhov' via Foreman users wrote: > One more thing I noticed in 1.10-rc - even though APIv2 is a default one > these days, discovered_hosts does not work unless "v2" is explicitly > specified: > > [root@centos7 ~]# curl -kSs -u admin: > https://localhost/api/*v2*/discovered_hosts | jq '.total' > 2 > > [root@centos7 ~]# curl -kSs -u admin: > https://localhost/api/discovered_hosts | jq '.total' > parse error: Invalid numeric literal at line 1, column 10 > > [root@centos7 ~]# curl -kSs -u admin: > https://localhost/api/discovered_hosts > > > > The page you were looking for doesn't exist (404)

–
Dominic Cleal
dominic@cleal.org

Jeff · November 20, 2015, 11:40am

I have just checked and the issue reported in #2369 is still a problem in
1.10RC3 and our workaround no longer works because of the removal of the
DELETE method on the sc parameter API. The only thing I can think of doing
at the moment is for us to manually re-enable the DELETE method in our
deployments until #2369 is fixed. Are there any other options?

I think this is a bit of a limitation of the current foreman architecture
whereby it is assumed that classes with the same name in different
environments are the same class (with the same parameters). I think it
makes more sense for them to be considered completely separate.

Jeff

···

On Wednesday, 18 November 2015 14:18:18 UTC, Jeff wrote: > > > On Wednesday, 18 November 2015 13:57:12 UTC, Dominic Cleal wrote: >> >> That's deliberate to match the UI, see "Disallow removal of smart class >> parameters by a user" in the 1.10 release notes >> (http://theforeman.org/manuals/1.10/index.html#Releasenotesfor1.10.0). >> >> The intention, like the earlier removal of the deletion of Puppet >> classes is for it to work by import only. >> > > Ouch, this causes a huge problem for us. Because of issue #2369 we have a > workaround using the API which basically deletes the sc parameter (from all > environments, since they appear to be global entities), then re-adds them > in the relevant environments by calling 'import puppetclasses'. Without the > delete method we cannot perform this workaround and puppet will fail on > run. > > Are there plans to fix #2369? >

Dominic_Cleal2 · November 24, 2015, 12:45pm

You've probably seen my Redmine updates, but for the record I've
submitted a patch to try and fix ticket #7517. I think #2369 is only
when a class has parameters and they're all removed.

#7517 and your new ticket #12579 seem to describe a specific issue where
the parameter isn't disassociated from an environment when it's marked
as overridden (non-overridden parameters should be removed fine as-is).
My fix should address this by removing the conditional that checked if
it was overridden.

If you get the chance to see if it fixes your import issue, it'd be much
appreciated. Please add a comment to the PR or ticket so it's seen.

Cheers,

···

On 18/11/15 14:18, Jeff wrote: > > On Wednesday, 18 November 2015 13:57:12 UTC, Dominic Cleal wrote: > > That's deliberate to match the UI, see "Disallow removal of smart class > parameters by a user" in the 1.10 release notes > (http://theforeman.org/manuals/1.10/index.html#Releasenotesfor1.10.0 > ). > > The intention, like the earlier removal of the deletion of Puppet > classes is for it to work by import only. > > > Ouch, this causes a huge problem for us. Because of issue #2369 we have > a workaround using the API which basically deletes the sc parameter > (from all environments, since they appear to be global entities), then > re-adds them in the relevant environments by calling 'import > puppetclasses'. Without the delete method we cannot perform this > workaround and puppet will fail on run. > > Are there plans to fix #2369?

–
Dominic Cleal
dominic@cleal.org

Konstantin_Orekhov · November 19, 2015, 10:25pm

> Could you file this as a bug against Discovery please? I think it might
> need a change to the plugin's routing configuration to state that API
> v2's the default when there's no version. (The plugin also ships no v1
> API.)

Here you go - Bug #12539: Discovery plug-in requires api/v2 to be specified even though v2 is the default in Foreman now - Discovery - Foreman
Thanks!

Jeff · November 24, 2015, 10:01am

Another update on this, it still fails, even if we manually re-enable the
delete API. I have raised a bug for this, its a pretty big problem for us…

http://projects.theforeman.org/issues/12579

Let me know if I can help in resolving this.
Jeff

···

On 20 November 2015 at 11:40, Jeff wrote:

On Wednesday, 18 November 2015 14:18:18 UTC, Jeff wrote:

On Wednesday, 18 November 2015 13:57:12 UTC, Dominic Cleal wrote:

That’s deliberate to match the UI, see “Disallow removal of smart class
parameters by a user” in the 1.10 release notes
(Foreman :: Manual).

The intention, like the earlier removal of the deletion of Puppet
classes is for it to work by import only.

Ouch, this causes a huge problem for us. Because of issue #2369 we have a
workaround using the API which basically deletes the sc parameter (from all
environments, since they appear to be global entities), then re-adds them
in the relevant environments by calling ‘import puppetclasses’. Without the
delete method we cannot perform this workaround and puppet will fail on
run.

Are there plans to fix #2369?

I have just checked and the issue reported in #2369 is still a problem in
1.10RC3 and our workaround no longer works because of the removal of the
DELETE method on the sc parameter API. The only thing I can think of doing
at the moment is for us to manually re-enable the DELETE method in our
deployments until #2369 is fixed. Are there any other options?

I think this is a bit of a limitation of the current foreman architecture
whereby it is assumed that classes with the same name in different
environments are the same class (with the same parameters). I think it
makes more sense for them to be considered completely separate.

Jeff

–
You received this message because you are subscribed to a topic in the
Google Groups “Foreman users” group.
To unsubscribe from this topic, visit
https://groups.google.com/d/topic/foreman-users/sYKgcASNUkg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-users+unsubscribe@googlegroups.com.
To post to this group, send email to foreman-users@googlegroups.com.
Visit this group at http://groups.google.com/group/foreman-users.
For more options, visit https://groups.google.com/d/optout.

Jeff · November 24, 2015, 1:58pm

Tried applying the patch to ~foreman/app/services/puppet_class_importer.rb
and restarting httpd but not seeing any change in behaviour. Am I missing
something?

···

On 24 November 2015 at 12:45, Dominic Cleal wrote:

On 18/11/15 14:18, Jeff wrote:
On Wednesday, 18 November 2015 13:57:12 UTC, Dominic Cleal wrote:
That's deliberate to match the UI, see "Disallow removal of smart
class
parameters by a user" in the 1.10 release notes
(http://theforeman.org/manuals/1.10/index.html#Releasenotesfor1.10.0
<http://theforeman.org/manuals/1.10/index.html#Releasenotesfor1.10.0
).
The intention, like the earlier removal of the deletion of Puppet
classes is for it to work by import only.
Ouch, this causes a huge problem for us. Because of issue #2369 we have
a workaround using the API which basically deletes the sc parameter
(from all environments, since they appear to be global entities), then
re-adds them in the relevant environments by calling ‘import
puppetclasses’. Without the delete method we cannot perform this
workaround and puppet will fail on run.

Are there plans to fix #2369?
You’ve probably seen my Redmine updates, but for the record I’ve
submitted a patch to try and fix ticket #7517. I think #2369 is only
when a class has parameters and they’re all removed.

#7517 and your new ticket #12579 seem to describe a specific issue where
the parameter isn’t disassociated from an environment when it’s marked
as overridden (non-overridden parameters should be removed fine as-is).
My fix should address this by removing the conditional that checked if
it was overridden.

If you get the chance to see if it fixes your import issue, it’d be much
appreciated. Please add a comment to the PR or ticket so it’s seen.

Cheers,

–
Dominic Cleal
dominic@cleal.org

–
You received this message because you are subscribed to a topic in the
Google Groups “Foreman users” group.
To unsubscribe from this topic, visit
https://groups.google.com/d/topic/foreman-users/sYKgcASNUkg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-users+unsubscribe@googlegroups.com.
To post to this group, send email to foreman-users@googlegroups.com.
Visit this group at http://groups.google.com/group/foreman-users.
For more options, visit https://groups.google.com/d/optout.

Dominic_Cleal2 · November 24, 2015, 2:05pm

What are you trying exactly? What it should do is to remove an
overridden smart class parameter from an environment when removed from a
class definition. It won't do anything without running a class import.

You should see the class get an "update" in the import to indicate the
parameter list has changed.

···

On 24/11/15 13:58, Jeff Sault wrote: > Tried applying the patch to > ~foreman/app/services/puppet_class_importer.rb and restarting httpd but > not seeing any change in behaviour. Am I missing something?

–
Dominic Cleal
dominic@cleal.org

Jeff · November 26, 2015, 11:59am

Sorry for the late reply, I've been trying to figure out the cause of my
issue. The fix provided does work but because of our internal tooling
around foreman I am seeing some weird issues which are unrelated.

Our internal tooling creates symlinks to link different versions of our
puppet modules into different puppet environments…

[root@jeff-fm110:/etc/puppet/environments/production/modules] # ll
total 4
lrwxrwxrwx 1 root root 46 Nov 18 10:26 mlds_server ->
/usr/share/nagra-foreman-installer/mlds_server
lrwxrwxrwx 1 root root 72 Nov 17 11:49 nsp_mlds ->
/usr/share/nagra-puppet-modules/NSP-MLDS-1.2STD2.20151109180714/nsp_mlds
lrwxrwxrwx 1 root root 53 Nov 26 11:49 test94120 ->
/usr/share/nagra-puppet-modules/test94120-2/test94120

In some cases, unless I touch my class files (eg init.pp) no changes are
detected when I call the import_puppetclasses API. How does the puppet
smart proxy know when a puppet class has changed?

Thanks again,
Jeff

···

On 24 November 2015 at 14:05, Dominic Cleal wrote:

On 24/11/15 13:58, Jeff Sault wrote:

Tried applying the patch to
~foreman/app/services/puppet_class_importer.rb and restarting httpd but
not seeing any change in behaviour. Am I missing something?

What are you trying exactly? What it should do is to remove an
overridden smart class parameter from an environment when removed from a
class definition. It won’t do anything without running a class import.

You should see the class get an “update” in the import to indicate the
parameter list has changed.

–
Dominic Cleal
dominic@cleal.org

–
You received this message because you are subscribed to a topic in the
Google Groups “Foreman users” group.
To unsubscribe from this topic, visit
https://groups.google.com/d/topic/foreman-users/sYKgcASNUkg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-users+unsubscribe@googlegroups.com.
To post to this group, send email to foreman-users@googlegroups.com.
Visit this group at http://groups.google.com/group/foreman-users.
For more options, visit https://groups.google.com/d/optout.

Dominic_Cleal2 · November 26, 2015, 12:50pm

> Sorry for the late reply, I've been trying to figure out the cause of my
> issue. The fix provided does work but because of our internal tooling
> around foreman I am seeing some weird issues which are unrelated.

That's good news, thanks.

> Our internal tooling creates symlinks to link different versions of our
> puppet modules into different puppet environments…
>
> [root@jeff-fm110:/etc/puppet/environments/production/modules] # ll
> total 4
> lrwxrwxrwx 1 root root 46 Nov 18 10:26 mlds_server ->
> /usr/share/nagra-foreman-installer/mlds_server
> lrwxrwxrwx 1 root root 72 Nov 17 11:49 nsp_mlds ->
> /usr/share/nagra-puppet-modules/NSP-MLDS-1.2STD2.20151109180714/nsp_mlds
> lrwxrwxrwx 1 root root 53 Nov 26 11:49 test94120 ->
> /usr/share/nagra-puppet-modules/test94120-2/test94120
>
>
> In some cases, unless I touch my class files (eg init.pp) no changes
> are detected when I call the import_puppetclasses API. How does the
> puppet smart proxy know when a puppet class has changed?

You may be seeing the effect of an in-memory cache in the smart proxy
process itself which is based on mtimes of the files. You can disable
this in /etc/foreman-proxy/settings.d/puppet.yml (:use_cache: false).

···

On 26/11/15 11:59, Jeff Sault wrote:

–
Dominic Cleal
dominic@cleal.org

Jeff · November 26, 2015, 1:23pm

Yep, thats it. I'll probably update our internal tools to touch the files
when they are linked in. Disabling the caching slows the import down
significantly.

Thanks for your help!
Jeff

···

On 26 November 2015 at 12:50, Dominic Cleal wrote:

On 26/11/15 11:59, Jeff Sault wrote:

Sorry for the late reply, I’ve been trying to figure out the cause of my
issue. The fix provided does work but because of our internal tooling
around foreman I am seeing some weird issues which are unrelated.

That’s good news, thanks.

Our internal tooling creates symlinks to link different versions of our
puppet modules into different puppet environments…

[root@jeff-fm110:/etc/puppet/environments/production/modules] # ll
total 4
lrwxrwxrwx 1 root root 46 Nov 18 10:26 mlds_server ->
/usr/share/nagra-foreman-installer/mlds_server
lrwxrwxrwx 1 root root 72 Nov 17 11:49 nsp_mlds ->
/usr/share/nagra-puppet-modules/NSP-MLDS-1.2STD2.20151109180714/nsp_mlds
lrwxrwxrwx 1 root root 53 Nov 26 11:49 test94120 ->
/usr/share/nagra-puppet-modules/test94120-2/test94120

In some cases, unless I touch my class files (eg init.pp) no changes
are detected when I call the import_puppetclasses API. How does the
puppet smart proxy know when a puppet class has changed?

You may be seeing the effect of an in-memory cache in the smart proxy
process itself which is based on mtimes of the files. You can disable
this in /etc/foreman-proxy/settings.d/puppet.yml (:use_cache: false).

–
Dominic Cleal
dominic@cleal.org

–
You received this message because you are subscribed to a topic in the
Google Groups “Foreman users” group.
To unsubscribe from this topic, visit
https://groups.google.com/d/topic/foreman-users/sYKgcASNUkg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-users+unsubscribe@googlegroups.com.
To post to this group, send email to foreman-users@googlegroups.com.
Visit this group at http://groups.google.com/group/foreman-users.
For more options, visit https://groups.google.com/d/optout.