Skip to content

Conversation

@ciprianbadescu
Copy link
Contributor

Before puppet-18.0, using an absolute path as package name allowed
users to install local packages using apt. While this was working,
the functionality was unintended and incomplete(eg. puppet will execute
package install command each run and will report changes).

This commit enables the usage of source parameter with apt provider
and now a user can install a local package using a manifest like bellow:

package { 'helloworld':
    source => '/tmp/helloworld_1.0-1.deb'
    ensure => installed,
}

Comment on lines 150 to 156
cmd << :install

if should_use_source?
cmd << @resource[:source]
else
cmd << str
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to provide some info if the manifest has ensure => 1.2.3, source => /path/to/packge. In this case i would assume the version is ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added check to allow only :present and :installed for ensure when using source parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could not be good enough since we may need a way to handle package upgrades

Copy link
Contributor Author

@ciprianbadescu ciprianbadescu Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we pass through ensure value and in case source file was specified, we make sure the expected version was installed (this is what yum provider does in all cases):

# If a source file was specified, we must make sure the expected version was installed from specified file
if source
is = self.query
raise Puppet::Error, _("Could not find package %{name}") % { name: self.name } unless is
version = is[:ensure]
raise Puppet::Error, _("Failed to update to version %{should}, got version %{version} instead") % { should: should, version: version } unless
insync?(version)
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could work and it would be consistent.

@ciprianbadescu ciprianbadescu force-pushed the PUP-10854/apt-local-packages branch from 5b13c44 to 7c9b177 Compare January 27, 2021 13:19
@ciprianbadescu ciprianbadescu requested review from a team and gimmyxd January 27, 2021 13:21
@ciprianbadescu ciprianbadescu force-pushed the PUP-10854/apt-local-packages branch 2 times, most recently from c0e007a to 1d573d5 Compare January 27, 2021 14:41
@ciprianbadescu ciprianbadescu changed the base branch from main to 6.x January 29, 2021 08:54
end

# If a source file was specified, we must make sure the expected version was installed from specified file
if source && !should.is_a?(Symbol)
Copy link
Contributor

@gimmyxd gimmyxd Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need !should.is_a?(Symbol)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the install command was successful, the package should have been installed, extra-check is not needed in case if :present or :installed values
we do extra-checks only in case specific version was requested

Copy link
Contributor

@gimmyxd gimmyxd Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than relying on object type, maybe we should check the actual values.

@ciprianbadescu ciprianbadescu force-pushed the PUP-10854/apt-local-packages branch from 1d573d5 to 19f20d1 Compare January 29, 2021 11:42
Before puppet-18.0, using an absolute path as package name allowed
users to install local packages using apt. While this was working,
the functionality was unintended and incomplete(eg. puppet will execute
package install command each run and will report changes).

This commit enables the usage of `source` parameter with `apt` provider
and now a user can install a local package using a manifest like bellow:
```
package { 'helloworld':
    source => '/tmp/helloworld_1.0-1.deb'
    ensure => installed,
}
```
@ciprianbadescu ciprianbadescu force-pushed the PUP-10854/apt-local-packages branch from 19f20d1 to 3884b28 Compare January 29, 2021 12:17
@ciprianbadescu ciprianbadescu merged commit d429b6c into puppetlabs:6.x Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants