Merge ~silverdrake11/landscape-charm:update_apt_lib into landscape-charm:main
- Git
- lp:~silverdrake11/landscape-charm
- update_apt_lib
- Merge into main
Proposed by
Kevin Nasto
Status: | Merged |
---|---|
Merged at revision: | 14c34233b846d584ba47b2ce3ba75d6d6722c4e7 |
Proposed branch: | ~silverdrake11/landscape-charm:update_apt_lib |
Merge into: | landscape-charm:main |
Diff against target: |
340 lines (+90/-59) 1 file modified
lib/charms/operator_libs_linux/v0/apt.py (+90/-59) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mitch Burton | Approve | ||
Review via email: mp+442181@code.launchpad.net |
Commit message
Applied latest update to apt library, which fixes the noninteractive mode issue some were experiencing
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/charms/operator_libs_linux/v0/apt.py b/lib/charms/operator_libs_linux/v0/apt.py |
2 | index 2f921f0..7afb183 100644 |
3 | --- a/lib/charms/operator_libs_linux/v0/apt.py |
4 | +++ b/lib/charms/operator_libs_linux/v0/apt.py |
5 | @@ -78,7 +78,6 @@ Keys are constructed as `{repo_type}-{}-{release}` in order to uniquely identify |
6 | Repositories can be added with explicit values through a Python constructor. |
7 | |
8 | Example: |
9 | - |
10 | ```python |
11 | repositories = apt.RepositoryMapping() |
12 | |
13 | @@ -91,7 +90,6 @@ Alternatively, any valid `sources.list` line may be used to construct a new |
14 | `DebianRepository`. |
15 | |
16 | Example: |
17 | - |
18 | ```python |
19 | repositories = apt.RepositoryMapping() |
20 | |
21 | @@ -124,7 +122,7 @@ LIBAPI = 0 |
22 | |
23 | # Increment this PATCH version before using `charmcraft publish-lib` or reset |
24 | # to 0 if you are raising the major API version |
25 | -LIBPATCH = 8 |
26 | +LIBPATCH = 11 |
27 | |
28 | |
29 | VALID_SOURCE_TYPES = ("deb", "deb-src") |
30 | @@ -135,7 +133,7 @@ class Error(Exception): |
31 | """Base class of most errors raised by this library.""" |
32 | |
33 | def __repr__(self): |
34 | - """String representation of Error.""" |
35 | + """Represent the Error.""" |
36 | return "<{}.{} {}>".format(type(self).__module__, type(self).__name__, self.args) |
37 | |
38 | @property |
39 | @@ -212,15 +210,15 @@ class DebianPackage: |
40 | ) == (other._name, other._version.number) |
41 | |
42 | def __hash__(self): |
43 | - """A basic hash so this class can be used in Mappings and dicts.""" |
44 | + """Return a hash of this package.""" |
45 | return hash((self._name, self._version.number)) |
46 | |
47 | def __repr__(self): |
48 | - """A representation of the package.""" |
49 | + """Represent the package.""" |
50 | return "<{}.{}: {}>".format(self.__module__, self.__class__.__name__, self.__dict__) |
51 | |
52 | def __str__(self): |
53 | - """A human-readable representation of the package.""" |
54 | + """Return a human-readable representation of the package.""" |
55 | return "<{}: {}-{}.{} -- {}>".format( |
56 | self.__class__.__name__, |
57 | self._name, |
58 | @@ -250,7 +248,8 @@ class DebianPackage: |
59 | package_names = [package_names] |
60 | _cmd = ["apt-get", "-y", *optargs, command, *package_names] |
61 | try: |
62 | - env = {"DEBIAN_FRONTEND": "noninteractive"} |
63 | + env = os.environ.copy() |
64 | + env["DEBIAN_FRONTEND"] = "noninteractive" |
65 | check_call(_cmd, env=env, stderr=PIPE, stdout=PIPE) |
66 | except CalledProcessError as e: |
67 | raise PackageError( |
68 | @@ -266,7 +265,7 @@ class DebianPackage: |
69 | ) |
70 | |
71 | def _remove(self) -> None: |
72 | - """Removes a package from the system. Implementation-specific.""" |
73 | + """Remove a package from the system. Implementation-specific.""" |
74 | return self._apt("remove", "{}={}".format(self.name, self.version)) |
75 | |
76 | @property |
77 | @@ -275,7 +274,7 @@ class DebianPackage: |
78 | return self._name |
79 | |
80 | def ensure(self, state: PackageState): |
81 | - """Ensures that a package is in a given state. |
82 | + """Ensure that a package is in a given state. |
83 | |
84 | Args: |
85 | state: a `PackageState` to reconcile the package to |
86 | @@ -307,7 +306,7 @@ class DebianPackage: |
87 | |
88 | @state.setter |
89 | def state(self, state: PackageState) -> None: |
90 | - """Sets the package state to a given value. |
91 | + """Set the package state to a given value. |
92 | |
93 | Args: |
94 | state: a `PackageState` to reconcile the package to |
95 | @@ -356,7 +355,7 @@ class DebianPackage: |
96 | |
97 | Args: |
98 | package: a string representing the package |
99 | - version: an optional string if a specific version isr equested |
100 | + version: an optional string if a specific version is requested |
101 | arch: an optional architecture, defaulting to `dpkg --print-architecture`. If an |
102 | architecture is not specified, this will be used for selection. |
103 | |
104 | @@ -389,7 +388,7 @@ class DebianPackage: |
105 | |
106 | Args: |
107 | package: a string representing the package |
108 | - version: an optional string if a specific version isr equested |
109 | + version: an optional string if a specific version is requested |
110 | arch: an optional architecture, defaulting to `dpkg --print-architecture`. |
111 | If an architecture is not specified, this will be used for selection. |
112 | """ |
113 | @@ -459,7 +458,7 @@ class DebianPackage: |
114 | |
115 | Args: |
116 | package: a string representing the package |
117 | - version: an optional string if a specific version isr equested |
118 | + version: an optional string if a specific version is requested |
119 | arch: an optional architecture, defaulting to `dpkg --print-architecture`. |
120 | If an architecture is not specified, this will be used for selection. |
121 | """ |
122 | @@ -515,7 +514,7 @@ class Version: |
123 | """An abstraction around package versions. |
124 | |
125 | This seems like it should be strictly unnecessary, except that `apt_pkg` is not usable inside a |
126 | - venv, and wedging version comparisions into `DebianPackage` would overcomplicate it. |
127 | + venv, and wedging version comparisons into `DebianPackage` would overcomplicate it. |
128 | |
129 | This class implements the algorithm found here: |
130 | https://www.debian.org/doc/debian-policy/ch-controlfields.html#version |
131 | @@ -526,11 +525,11 @@ class Version: |
132 | self._epoch = epoch or "" |
133 | |
134 | def __repr__(self): |
135 | - """A representation of the package.""" |
136 | + """Represent the package.""" |
137 | return "<{}.{}: {}>".format(self.__module__, self.__class__.__name__, self.__dict__) |
138 | |
139 | def __str__(self): |
140 | - """A human-readable representation of the package.""" |
141 | + """Return human-readable representation of the package.""" |
142 | return "{}{}".format("{}:".format(self._epoch) if self._epoch else "", self._version) |
143 | |
144 | @property |
145 | @@ -731,13 +730,16 @@ def add_package( |
146 | """Add a package or list of packages to the system. |
147 | |
148 | Args: |
149 | + package_names: single package name, or list of package names |
150 | name: the name(s) of the package(s) |
151 | version: an (Optional) version as a string. Defaults to the latest known |
152 | arch: an optional architecture for the package |
153 | update_cache: whether or not to run `apt-get update` prior to operating |
154 | |
155 | Raises: |
156 | + TypeError if no package name is given, or explicit version is set for multiple packages |
157 | PackageNotFoundError if the package is not in the cache. |
158 | + PackageError if packages fail to install |
159 | """ |
160 | cache_refreshed = False |
161 | if update_cache: |
162 | @@ -785,7 +787,7 @@ def _add( |
163 | version: Optional[str] = "", |
164 | arch: Optional[str] = "", |
165 | ) -> Tuple[Union[DebianPackage, str], bool]: |
166 | - """Adds a package. |
167 | + """Add a package to the system. |
168 | |
169 | Args: |
170 | name: the name(s) of the package(s) |
171 | @@ -806,7 +808,7 @@ def _add( |
172 | def remove_package( |
173 | package_names: Union[str, List[str]] |
174 | ) -> Union[DebianPackage, List[DebianPackage]]: |
175 | - """Removes a package from the system. |
176 | + """Remove package(s) from the system. |
177 | |
178 | Args: |
179 | package_names: the name of a package |
180 | @@ -834,10 +836,72 @@ def remove_package( |
181 | |
182 | |
183 | def update() -> None: |
184 | - """Updates the apt cache via `apt-get update`.""" |
185 | + """Update the apt cache via `apt-get update`.""" |
186 | check_call(["apt-get", "update"], stderr=PIPE, stdout=PIPE) |
187 | |
188 | |
189 | +def import_key(key: str) -> str: |
190 | + """Import an ASCII Armor key. |
191 | + |
192 | + A Radix64 format keyid is also supported for backwards |
193 | + compatibility. In this case Ubuntu keyserver will be |
194 | + queried for a key via HTTPS by its keyid. This method |
195 | + is less preferable because https proxy servers may |
196 | + require traffic decryption which is equivalent to a |
197 | + man-in-the-middle attack (a proxy server impersonates |
198 | + keyserver TLS certificates and has to be explicitly |
199 | + trusted by the system). |
200 | + |
201 | + Args: |
202 | + key: A GPG key in ASCII armor format, including BEGIN |
203 | + and END markers or a keyid. |
204 | + |
205 | + Returns: |
206 | + The GPG key filename written. |
207 | + |
208 | + Raises: |
209 | + GPGKeyError if the key could not be imported |
210 | + """ |
211 | + key = key.strip() |
212 | + if "-" in key or "\n" in key: |
213 | + # Send everything not obviously a keyid to GPG to import, as |
214 | + # we trust its validation better than our own. eg. handling |
215 | + # comments before the key. |
216 | + logger.debug("PGP key found (looks like ASCII Armor format)") |
217 | + if ( |
218 | + "-----BEGIN PGP PUBLIC KEY BLOCK-----" in key |
219 | + and "-----END PGP PUBLIC KEY BLOCK-----" in key |
220 | + ): |
221 | + logger.debug("Writing provided PGP key in the binary format") |
222 | + key_bytes = key.encode("utf-8") |
223 | + key_name = DebianRepository._get_keyid_by_gpg_key(key_bytes) |
224 | + key_gpg = DebianRepository._dearmor_gpg_key(key_bytes) |
225 | + gpg_key_filename = "/etc/apt/trusted.gpg.d/{}.gpg".format(key_name) |
226 | + DebianRepository._write_apt_gpg_keyfile( |
227 | + key_name=gpg_key_filename, key_material=key_gpg |
228 | + ) |
229 | + return gpg_key_filename |
230 | + else: |
231 | + raise GPGKeyError("ASCII armor markers missing from GPG key") |
232 | + else: |
233 | + logger.warning( |
234 | + "PGP key found (looks like Radix64 format). " |
235 | + "SECURELY importing PGP key from keyserver; " |
236 | + "full key not provided." |
237 | + ) |
238 | + # as of bionic add-apt-repository uses curl with an HTTPS keyserver URL |
239 | + # to retrieve GPG keys. `apt-key adv` command is deprecated as is |
240 | + # apt-key in general as noted in its manpage. See lp:1433761 for more |
241 | + # history. Instead, /etc/apt/trusted.gpg.d is used directly to drop |
242 | + # gpg |
243 | + key_asc = DebianRepository._get_key_by_keyid(key) |
244 | + # write the key in GPG format so that apt-key list shows it |
245 | + key_gpg = DebianRepository._dearmor_gpg_key(key_asc.encode("utf-8")) |
246 | + gpg_key_filename = "/etc/apt/trusted.gpg.d/{}.gpg".format(key) |
247 | + DebianRepository._write_apt_gpg_keyfile(key_name=gpg_key_filename, key_material=key_gpg) |
248 | + return gpg_key_filename |
249 | + |
250 | + |
251 | class InvalidSourceError(Error): |
252 | """Exceptions for invalid source entries.""" |
253 | |
254 | @@ -901,7 +965,7 @@ class DebianRepository: |
255 | |
256 | @filename.setter |
257 | def filename(self, fname: str) -> None: |
258 | - """Sets the filename used when a repo is written back to diskself. |
259 | + """Set the filename used when a repo is written back to disk. |
260 | |
261 | Args: |
262 | fname: a filename to write the repository information to. |
263 | @@ -1004,7 +1068,7 @@ class DebianRepository: |
264 | A Radix64 format keyid is also supported for backwards |
265 | compatibility. In this case Ubuntu keyserver will be |
266 | queried for a key via HTTPS by its keyid. This method |
267 | - is less preferrable because https proxy servers may |
268 | + is less preferable because https proxy servers may |
269 | require traffic decryption which is equivalent to a |
270 | man-in-the-middle attack (a proxy server impersonates |
271 | keyserver TLS certificates and has to be explicitly |
272 | @@ -1017,40 +1081,7 @@ class DebianRepository: |
273 | Raises: |
274 | GPGKeyError if the key could not be imported |
275 | """ |
276 | - key = key.strip() |
277 | - if "-" in key or "\n" in key: |
278 | - # Send everything not obviously a keyid to GPG to import, as |
279 | - # we trust its validation better than our own. eg. handling |
280 | - # comments before the key. |
281 | - logger.debug("PGP key found (looks like ASCII Armor format)") |
282 | - if ( |
283 | - "-----BEGIN PGP PUBLIC KEY BLOCK-----" in key |
284 | - and "-----END PGP PUBLIC KEY BLOCK-----" in key |
285 | - ): |
286 | - logger.debug("Writing provided PGP key in the binary format") |
287 | - key_bytes = key.encode("utf-8") |
288 | - key_name = self._get_keyid_by_gpg_key(key_bytes) |
289 | - key_gpg = self._dearmor_gpg_key(key_bytes) |
290 | - self._gpg_key_filename = "/etc/apt/trusted.gpg.d/{}.gpg".format(key_name) |
291 | - self._write_apt_gpg_keyfile(key_name=self._gpg_key_filename, key_material=key_gpg) |
292 | - else: |
293 | - raise GPGKeyError("ASCII armor markers missing from GPG key") |
294 | - else: |
295 | - logger.warning( |
296 | - "PGP key found (looks like Radix64 format). " |
297 | - "SECURELY importing PGP key from keyserver; " |
298 | - "full key not provided." |
299 | - ) |
300 | - # as of bionic add-apt-repository uses curl with an HTTPS keyserver URL |
301 | - # to retrieve GPG keys. `apt-key adv` command is deprecated as is |
302 | - # apt-key in general as noted in its manpage. See lp:1433761 for more |
303 | - # history. Instead, /etc/apt/trusted.gpg.d is used directly to drop |
304 | - # gpg |
305 | - key_asc = self._get_key_by_keyid(key) |
306 | - # write the key in GPG format so that apt-key list shows it |
307 | - key_gpg = self._dearmor_gpg_key(key_asc.encode("utf-8")) |
308 | - self._gpg_key_filename = "/etc/apt/trusted.gpg.d/{}.gpg".format(key) |
309 | - self._write_apt_gpg_keyfile(key_name=key, key_material=key_gpg) |
310 | + self._gpg_key_filename = import_key(key) |
311 | |
312 | @staticmethod |
313 | def _get_keyid_by_gpg_key(key_material: bytes) -> str: |
314 | @@ -1116,7 +1147,7 @@ class DebianRepository: |
315 | |
316 | @staticmethod |
317 | def _dearmor_gpg_key(key_asc: bytes) -> bytes: |
318 | - """Converts a GPG key in the ASCII armor format to the binary format. |
319 | + """Convert a GPG key in the ASCII armor format to the binary format. |
320 | |
321 | Args: |
322 | key_asc: A GPG key in ASCII armor format. |
323 | @@ -1140,7 +1171,7 @@ class DebianRepository: |
324 | |
325 | @staticmethod |
326 | def _write_apt_gpg_keyfile(key_name: str, key_material: bytes) -> None: |
327 | - """Writes GPG key material into a file at a provided path. |
328 | + """Write GPG key material into a file at a provided path. |
329 | |
330 | Args: |
331 | key_name: A key name to use for a key file (could be a fingerprint) |
332 | @@ -1188,7 +1219,7 @@ class RepositoryMapping(Mapping): |
333 | return len(self._repository_map) |
334 | |
335 | def __iter__(self) -> Iterable[DebianRepository]: |
336 | - """Iterator magic method for RepositoryMapping.""" |
337 | + """Return iterator for RepositoryMapping.""" |
338 | return iter(self._repository_map.values()) |
339 | |
340 | def __getitem__(self, repository_uri: str) -> DebianRepository: |
+1 LGTM