Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement using deb822 format in source application add command #457

Merged
merged 14 commits into from
Jan 10, 2024
34 changes: 24 additions & 10 deletions docs/Manifest Parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -918,10 +918,10 @@ The query command can be used to gather information about the system and the Vis
| `<type></type>` | `<type>source</type>` | R | |
| `<applicationSource>` | `<applicationSource>` | R | |
| `<add>` | `<add>` | R | |
| `<gpg>` | `<gpg>` | R | |
| `<uri></uri>` | `<uri>https://dl-ssl.google.com/linux/linux_signing_key.pub</uri>` | R | |
| `<keyname></keyname>` | `<keyname>google-chrome.gpg</keyname>` | R | |
| `</gpg>` | `</gpg>` | R | |
| `<gpg>` | `<gpg>` | O | |
| `<uri></uri>` | `<uri>https://dl-ssl.google.com/linux/linux_signing_key.pub</uri>` | O | |
| `<keyname></keyname>` | `<keyname>google-chrome.gpg</keyname>` | O | |
| `</gpg>` | `</gpg>` | O | |
| `<repo>` | `<repo>` | R | |
| `<repos>` | `<repos>` | R | |
| `<source_pkg>` | `<source_pkg>deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main</source_pkg>` | R | |
Expand All @@ -935,7 +935,7 @@ The query command can be used to gather information about the system and the Vis



#### Source application add Manifest Example
#### Source Application Add Manifest Example using remote GPG key
```xml
<?xml version="1.0" encoding="UTF-8"?>
<manifest>
Expand Down Expand Up @@ -966,7 +966,6 @@ The query command can be used to gather information about the system and the Vis
| `<type></type>` | `<type>source</type>` | R | |
| `<applicationSource>` | `<applicationSource>` | R | |
| `<update>` | `<update>` | R | |
| `<gpg>` | `<gpg>` | R | |
| `<repo>` | `<repo>` | R | |
| `<repos>` | `<repos>` | R | |
| `<source_pkg>` | `<source_pkg>deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main</source_pkg>` | R | |
Expand Down Expand Up @@ -1006,9 +1005,9 @@ The query command can be used to gather information about the system and the Vis
| `<type></type>` | `<type>source</type>` | R | |
| `<applicationSource>` | `<applicationSource>` | R | |
| `<remove>` | `<remove>` | R | |
| `<gpg>` | `<gpg>` | R | |
| `<keyname></keyname>` | `<keyname>google-chrome.gpg</keyname>` | R | |
| `</gpg>` | `<gpg>` | R | |
| `<gpg>` | `<gpg>` | O | |
| `<keyname></keyname>` | `<keyname>google-chrome.gpg</keyname>` | O | |
| `</gpg>` | `<gpg>` | O | |
| `<repo>` | `<repo>` | R | |
| `<filename></filename>` | `<filename>google-chrom.list</filename>` | R | |
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
| `</repo>` | `</repo>` | R | |
Expand All @@ -1019,7 +1018,7 @@ The query command can be used to gather information about the system and the Vis



#### Source Application Remove Manifest Example
#### Source Application Remove Manifest Example (Including GPG key)
```xml
<?xml version="1.0" encoding="UTF-8"?>
<manifest>
Expand All @@ -1037,6 +1036,21 @@ The query command can be used to gather information about the system and the Vis
</manifest>
```

#### Source Application Remove Manifest Example (Excluding GPG key)
```xml
<?xml version="1.0" encoding="UTF-8"?>
<manifest>
<type>source</type>
<applicationSource>
<remove>
<repo>
<filename>google-chrome.list</filename>
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
</repo>
</remove>
</applicationSource>
</manifest>
```

#### Source Application List Manifest Parameters
| Tag | Example | Required/Optional | Notes |
|:-----------------------------------------|:-----------------------------------------|:-----------------:|:------|
Expand Down
78 changes: 66 additions & 12 deletions inbc-program/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,20 +414,20 @@ inbc query --option sw

## SOURCE APPLICATION ADD
### Description
Downloads and encrypts GPG key and stores it on the system under <em>/usr/share/keyrings</em>. Creates a file under <em>/etc/apt/sources.list.d</em> to store the update source information.
This list file is used during 'sudo apt update' to update the application
Optionally Downloads and encrypts GPG key and stores it on the system under <em>/usr/share/keyrings</em>. Creates a file under <em>/etc/apt/sources.list.d</em> to store the update source information.
This list file is used during 'sudo apt update' to update the application. <em>Deb882</em> format may be used instead of downloading a GPG key.

### Usage
```
inbc source application add
{--gpgKeyUri, -gku=GPG_KEY_URI}
{--gpgKeyName, -gkn=GPG_KEY_NAME}
inbc source application add
{--sources, -s=SOURCES}
{--filename, -f=FILENAME}
[--gpgKeyUri, -gku=GPG_KEY_URI]
[--gpgKeyName, -gkn=GPG_KEY_NAME]
```

### Example
#### Add an application source
#### Add an Application Source (non-deb822 format with remote GPG key)
```
inbc source application add
--gpgKeyUri https://dl-ssl.google.com/linux/linux_signing_key.pub
Expand All @@ -436,25 +436,81 @@ inbc source application add
--filename google-chrome.list
```

#### Add an Application Source (using deb822 format)

**NOTE:** In the Signed-By: Section, use the following guidelines.

- Each blank line has a period in it. -> " ."
- Each line after the Signed-By: starts with a space -> " gibberish"

```
inbc source application add
--sources
"Enabled: yes"
"Types: deb"
"URIs: http://dl.google.com/linux/chrome/deb/"
"Suites: stable"
"Components: main"
"Signed-By:"
" -----BEGIN PGP PUBLIC KEY BLOCK-----"
" Version: GnuPG v1.4.2.2 (GNU/Linux)"
" ."
" mQGiBEXwb0YRBADQva2NLpYXxgjNkbuP0LnPoEXruGmvi3XMIxjEUFuGNCP4Rj/a"
" kv2E5VixBP1vcQFDRJ+p1puh8NU0XERlhpyZrVMzzS/RdWdyXf7E5S8oqNXsoD1z"
" fvmI+i9b2EhHAA19Kgw7ifV8vMa4tkwslEmcTiwiw8lyUl28Wh4Et8SxzwCggDcA"
" feGqtn3PP5YAdD0km4S4XeMEAJjlrqPoPv2Gf//tfznY2UyS9PUqFCPLHgFLe80u"
" QhI2U5jt6jUKN4fHauvR6z3seSAsh1YyzyZCKxJFEKXCCqnrFSoh4WSJsbFNc4PN"
" b0V0SqiTCkWADZyLT5wll8sWuQ5ylTf3z1ENoHf+G3um3/wk/+xmEHvj9HCTBEXP"
" 78X0A/0Tqlhc2RBnEf+AqxWvM8sk8LzJI/XGjwBvKfXe+l3rnSR2kEAvGzj5Sg0X"
" 4XmfTg4Jl8BNjWyvm2Wmjfet41LPmYJKsux3g0b8yzQxeOA4pQKKAU3Z4+rgzGmf"
" HdwCG5MNT2A5XxD/eDd+L4fRx0HbFkIQoAi1J3YWQSiTk15fw7RMR29vZ2xlLCBJ"
" bmMuIExpbnV4IFBhY2thZ2UgU2lnbmluZyBLZXkgPGxpbnV4LXBhY2thZ2VzLWtl"
" eW1hc3RlckBnb29nbGUuY29tPohjBBMRAgAjAhsDBgsJCAcDAgQVAggDBBYCAwEC"
" HgECF4AFAkYVdn8CGQEACgkQoECDD3+sWZHKSgCfdq3HtNYJLv+XZleb6HN4zOcF"
" AJEAniSFbuv8V5FSHxeRimHx25671az+uQINBEXwb0sQCACuA8HT2nr+FM5y/kzI"
" A51ZcC46KFtIDgjQJ31Q3OrkYP8LbxOpKMRIzvOZrsjOlFmDVqitiVc7qj3lYp6U"
" rgNVaFv6Qu4bo2/ctjNHDDBdv6nufmusJUWq/9TwieepM/cwnXd+HMxu1XBKRVk9"
" XyAZ9SvfcW4EtxVgysI+XlptKFa5JCqFM3qJllVohMmr7lMwO8+sxTWTXqxsptJo"
" pZeKz+UBEEqPyw7CUIVYGC9ENEtIMFvAvPqnhj1GS96REMpry+5s9WKuLEaclWpd"
" K3krttbDlY1NaeQUCRvBYZ8iAG9YSLHUHMTuI2oea07Rh4dtIAqPwAX8xn36JAYG"
" 2vgLAAMFB/wKqaycjWAZwIe98Yt0qHsdkpmIbarD9fGiA6kfkK/UxjL/k7tmS4Vm"
" CljrrDZkPSQ/19mpdRcGXtb0NI9+nyM5trweTvtPw+HPkDiJlTaiCcx+izg79Fj9"
" KcofuNb3lPdXZb9tzf5oDnmm/B+4vkeTuEZJ//IFty8cmvCpzvY+DAz1Vo9rA+Zn"
" cpWY1n6z6oSS9AsyT/IFlWWBZZ17SpMHu+h4Bxy62+AbPHKGSujEGQhWq8ZRoJAT"
" G0KSObnmZ7FwFWu1e9XFoUCt0bSjiJWTIyaObMrWu/LvJ3e9I87HseSJStfw6fki"
" 5og9qFEkMrIrBCp3QGuQWBq/rTdMuwNFiEkEGBECAAkFAkXwb0sCGwwACgkQoECD"
" D3+sWZF/WACfeNAu1/1hwZtUo1bR+MWiCjpvHtwAnA1R3IHqFLQ2X3xJ40XPuAyY"
" /FJG"
" %20=Quqp"
" -----END PGP PUBLIC KEY BLOCK-----"
--filename google-chrome.sources
```

## SOURCE APPLICATION REMOVE
### Description
Removes the GPG key file from under <em>/usr/share/keyrings</em>. Removes the source file from under /etc/apt/sources.list.d/.
Removes the source file from under /etc/apt/sources.list.d/. Optionally removes the GPG key file from under <em>/usr/share/keyrings</em>.

### Usage
```
inbc source application remove
{--gpgKeyName, -gkn=GPG_KEY_NAME}
inbc source application remove
{--filename, -f=FILE_NAME}
[--gpgKeyName, -gkn=GPG_KEY_NAME]
```

### Example
#### Remove an application source
#### Remove an application source (Both GPG key and Source File)
```commandline
inbc source application remove
--gpgKeyName google-chrome.gpg
--filename google-chrome.list
```

#### Remove an application source (deb822 format)
```commandline
inbc source application remove
--filename google-chrome.sources
```

## SOURCE APPLICATION UPDATE
### Description
Updates Application sources that are used to update the system
Expand All @@ -478,7 +534,6 @@ inbc source application update
## SOURCE APPLICATION LIST
### Description
Lists Application sources
NOTE: Currently this only works on Ubuntu

### Usage
pillaiabhishek marked this conversation as resolved.
Show resolved Hide resolved
```
Expand Down Expand Up @@ -545,7 +600,6 @@ inbc source os update
## SOURCE OS LIST
### Description
Lists OS sources
NOTE: Currently this only works on Ubuntu

### Usage
```commandline
Expand Down
6 changes: 3 additions & 3 deletions inbc-program/inbc/parser/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ def parse_source_args(self) -> None:

# Application Add Command
app_add_parser = app_subparsers.add_parser('add')
app_add_parser.add_argument('--gpgKeyUri', '-gku', required=True,
app_add_parser.add_argument('--gpgKeyUri', '-gku', required=False,
type=lambda x: validate_string_less_than_n_characters(
x, 'str', 1500),
help='Uri from which to download GPG key')
app_add_parser.add_argument('--gpgKeyName', '-gkn', required=True,
app_add_parser.add_argument('--gpgKeyName', '-gkn', required=False,
type=lambda x: validate_string_less_than_n_characters(
x, 'str', 200),
help='Name to store the GPG key information')
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +68 to 75
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: The 'required' flag for '--gpgKeyUri' and '--gpgKeyName' arguments in the 'app_add_parser' has been changed from True to False. Ensure that the rest of the code properly handles cases where these arguments might not be provided, to prevent runtime errors. [possible issue]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
app_add_parser.add_argument('--gpgKeyUri', '-gku', required=False,
type=lambda x: validate_string_less_than_n_characters(
x, 'str', 1500),
help='Uri from which to download GPG key')
app_add_parser.add_argument('--gpgKeyName', '-gkn', required=True,
app_add_parser.add_argument('--gpgKeyName', '-gkn', required=False,
type=lambda x: validate_string_less_than_n_characters(
x, 'str', 200),
help='Name to store the GPG key information')
# It's assumed that the application_add function is designed to handle optional GPG key parameters.
# If not, the function should be updated accordingly to avoid runtime errors when these arguments are not provided.

Expand All @@ -85,7 +85,7 @@ def parse_source_args(self) -> None:

# Application Remove Command
app_remove_parser = app_subparsers.add_parser('remove')
app_remove_parser.add_argument('--gpgKeyName', '-gkn', required=True,
app_remove_parser.add_argument('--gpgKeyName', '-gkn', required=False,
type=lambda x: validate_string_less_than_n_characters(
x, 'str', 50),
help='GPG key name of the source to remove.')
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
40 changes: 26 additions & 14 deletions inbc-program/inbc/parser/source_app_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@
SPDX-License-Identifier: Apache-2.0
"""
import argparse
import logging

from ..inbc_exception import InbcException
from ..xml_tag import create_xml_tag

logger = logging.getLogger(__name__)


def application_add(args: argparse.Namespace) -> str:
if (args.gpgKeyUri and args.gpgKeyName is None) or (args.gpgKeyName and args.gpgKeyUri is None):
raise InbcException(
"Source requires either both gpgKeyUri and gpgKeyName to be provided, or neither of them.")
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
nmgaston marked this conversation as resolved.
Show resolved Hide resolved

arguments = {
'uri': args.gpgKeyUri,
'keyname': args.gpgKeyName,
Expand All @@ -19,15 +27,16 @@ def application_add(args: argparse.Namespace) -> str:
manifest = ('<?xml version="1.0" encoding="utf-8"?>' +
'<manifest>' +
'<type>source</type>' +
'<applicationSource>' +
'<add><gpg>'
'{0}' +
'{1}'
'</gpg><repo><repos>').format(create_xml_tag(arguments, "uri"),
create_xml_tag(arguments, "keyname"))
'<applicationSource>' + '<add>')
Comment on lines 27 to +30
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Replace the manual XML header creation with a constant to avoid repetition. [maintainability]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest = ('<?xml version="1.0" encoding="utf-8"?>' +
'<manifest>' +
'<type>source</type>' +
'<applicationSource>' +
'<add><gpg>'
'{0}' +
'{1}'
'</gpg><repo><repos>').format(create_xml_tag(arguments, "uri"),
create_xml_tag(arguments, "keyname"))
'<applicationSource>' + '<add>')
XML_HEADER = '<?xml version="1.0" encoding="utf-8"?>'
manifest = (f'{XML_HEADER}'
'<manifest><type>source</type>'
'<applicationSource><add>')


if args.gpgKeyUri and args.gpgKeyName:
manifest += ('<gpg>' + '{0}' + '{1}' + '</gpg>').format(create_xml_tag(arguments, "uri"),
create_xml_tag(arguments, "keyname"))
Comment on lines +33 to +34
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Avoid using string concatenation for XML generation to prevent potential XML injection vulnerabilities. [security]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest += ('<gpg>' + '{0}' + '{1}' + '</gpg>').format(create_xml_tag(arguments, "uri"),
create_xml_tag(arguments, "keyname"))
manifest += f'<gpg>{create_xml_tag(arguments, "uri")}{create_xml_tag(arguments, "keyname")}</gpg>'

Comment on lines +33 to +34
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Replace string concatenation with f-strings for better readability and performance. [enhancement]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest += ('<gpg>' + '{0}' + '{1}' + '</gpg>').format(create_xml_tag(arguments, "uri"),
create_xml_tag(arguments, "keyname"))
manifest += f'<gpg>{create_xml_tag(arguments, "uri")}{create_xml_tag(arguments, "keyname")}</gpg>'


manifest += '<repo><repos>'

for source in args.sources:
manifest += '<source_pkg>' + source.strip() + '</source_pkg>'
manifest += '<source_pkg>' + source + '</source_pkg>'
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines 38 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Use a list and join to build the XML string to improve performance. [performance]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
for source in args.sources:
manifest += '<source_pkg>' + source.strip() + '</source_pkg>'
manifest += '<source_pkg>' + source + '</source_pkg>'
sources_xml = ''.join(f'<source_pkg>{source}</source_pkg>' for source in args.sources)
manifest += f'<repo><repos>{sources_xml}</repos>'

Comment on lines 38 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Ensure that the source variable is stripped of leading and trailing whitespace before being added to the XML manifest. [bug]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
for source in args.sources:
manifest += '<source_pkg>' + source.strip() + '</source_pkg>'
manifest += '<source_pkg>' + source + '</source_pkg>'
for source in args.sources:
manifest += f'<source_pkg>{source.strip()}</source_pkg>'

nmgaston marked this conversation as resolved.
Show resolved Hide resolved

manifest += ('</repos>'
f'{create_xml_tag(arguments, "filename")}</repo>'
Comment on lines 41 to 42
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Remove unnecessary parentheses from the single-line f-string. [enhancement]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest += ('</repos>'
f'{create_xml_tag(arguments, "filename")}</repo>'
manifest += f'</repos>{create_xml_tag(arguments, "filename")}</repo></add></applicationSource></manifest>'

Expand All @@ -47,13 +56,16 @@ def application_remove(args: argparse.Namespace) -> str:
manifest = ('<?xml version="1.0" encoding="utf-8"?>' +
'<manifest><type>source</type>' +
'<applicationSource>' +
'<remove><gpg>'
f'{create_xml_tag(arguments, "keyname")}' +
'</gpg><repo>' +
f'{create_xml_tag(arguments, "filename")}'
'</repo>'
'</remove></applicationSource>' +
'</manifest>')
'<remove>')
Comment on lines 56 to +59
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Refactor the XML generation into a separate function to reduce code duplication and improve maintainability. [maintainability]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest = ('<?xml version="1.0" encoding="utf-8"?>' +
'<manifest><type>source</type>' +
'<applicationSource>' +
'<remove><gpg>'
f'{create_xml_tag(arguments, "keyname")}' +
'</gpg><repo>' +
f'{create_xml_tag(arguments, "filename")}'
'</repo>'
'</remove></applicationSource>' +
'</manifest>')
'<remove>')
def generate_manifest(tag_name, content):
return (f'<?xml version="1.0" encoding="utf-8"?>'
f'<manifest><type>source</type>'
f'<applicationSource>'
f'<{tag_name}>{content}</{tag_name}></applicationSource>'
f'</manifest>')
# Usage example:
manifest = generate_manifest('remove', content)


if args.gpgKeyName:
manifest += f'<gpg>{create_xml_tag(arguments, "keyname")}</gpg>'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Avoid using f-strings inside expressions where curly braces are used for XML tags, as it might cause confusion. Use format method or concatenate the strings for clarity. [readability]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
manifest += f'<gpg>{create_xml_tag(arguments, "keyname")}</gpg>'
manifest += '<gpg>' + create_xml_tag(arguments, "keyname") + '</gpg>'


manifest += ('<repo>' +
f'{create_xml_tag(arguments, "filename")}'
'</repo>'
'</remove></applicationSource>' +
'</manifest>')

print(f"manifest {manifest}")
nmgaston marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Use a context manager to handle the printing of the manifest to ensure that resources are properly managed. [best practice]
NOTE: Ensure that the suggested code is comprehensive. This serves as an example of the code you could use. The user should verify it after accepting the suggestion.

Suggested change
print(f"manifest {manifest}")
with open('manifest_log.txt', 'a') as log_file:
log_file.write(f"manifest {manifest}\n")

return manifest
Expand Down
Loading