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.

Sign up for GitHub

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

Jump to bottom

add krb5 auth params for exit_koji_promote (auth to koji with krb5) #482

Merged
merged 2 commits into from Dec 8, 2016
Merged

add krb5 auth params for exit_koji_promote (auth to koji with krb5) #482

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d9d0f6
add krb5 auth params for exit_koji_promote (auth to koji with krb5)
maxamillion Nov 30, 2016
b13d7f5
fix up conditional for setting koji_kerberos in build_request to veri…
maxamillion Dec 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/configuration_file.md
View file
Open in desktop
Expand Up @@ -53,6 +53,12 @@ Some options are also mandatory.

* `koji_certs_secret` (*optional*, `string`) — name of [kubernetes secret](https://github.com/kubernetes/kubernetes/blob/master/docs/design/secrets.md) to use for koji authentication

* `koji_use_kerberos` (*optional*, `boolean`) — will set [atomic-reactor](https://github.com/projectatomic/atomic-reactor) plugins to use kerberos to authenticate to koji.

* `koji_kerberos_keytab` (*optional*, `string`) - location of the keytab that will be used to initialize kerberos credentials for [atomic-reactor](https://github.com/projectatomic/atomic-reactor) plugins - usually in the form `FILE:<absolute_path>`, see [kerberos documentation](http://web.mit.edu/Kerberos/krb5-latest/doc/basic/keytab_def.html) for other possible values

* `koji_kerberos_principal` (*optional*, `string`) - kerberos principal for the keytab provided in `koji_kerberos_keytab`

* `sources_command` (*optional*, `string`) — command to use to get dist-git artifacts from lookaside cache (e.g. `fedpkg sources`)

* `username`, `password` (*optional*, `string`) — when OpenShift is hidden behind authentication proxy, you can specify username and password for basic authentication
Expand Down
4 changes: 3 additions & 1 deletion inputs/prod_inner.json
View file
Open in desktop
Expand Up @@ -154,7 +154,9 @@
"kojihub": "{{KOJI_HUB}}",
"url": "{{OPENSHIFT_URI}}",
"verify_ssl": false,
"blocksize": 10485760
"blocksize": 10485760,
"koji_keytab": false,
"koji_principal": false
}
},
{
Expand Down
3 changes: 3 additions & 0 deletions osbs/api.py
View file
Open in desktop
Expand Up @@ -393,6 +393,9 @@ def create_prod_build(self, git_uri, git_ref,
koji_target=target,
koji_certs_secret=self.build_conf.get_koji_certs_secret(),
koji_task_id=koji_task_id,
koji_use_kerberos=self.build_conf.get_koji_use_kerberos(),
koji_kerberos_keytab=self.build_conf.get_koji_kerberos_keytab(),
koji_kerberos_principal=self.build_conf.get_koji_kerberos_principal(),
architecture=architecture,
vendor=self.build_conf.get_vendor(),
build_host=self.build_conf.get_build_host(),
Expand Down
8 changes: 7 additions & 1 deletion osbs/build/build_request.py
View file
Open in desktop
Expand Up @@ -486,7 +486,13 @@ def render_koji_promote(self, use_auth=None):

if use_auth is not None:
self.dj.dock_json_set_arg('exit_plugins', 'koji_promote',
'use_auth', use_auth)
'use_auth', use_auth)

if self.spec.koji_use_kerberos.value:
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth enforcing that if koji_use_kerberos is set to True, principal and keytab values are also set? Alternatively, we could remove the koji_use_kerberos setting and only assume its behavior if koji_kerberos_principal and koji_kerberos_keytab are set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lcarva I'm good either way, the only reasdon I added koji_use_kerberos was there is a use_kerberos config flag already for the client and I was aiming for consistency. Which ever you and @twaugh (and Co.) prefer, I'm happy to go that direction.

Copy link
Member

Choose a reason for hiding this comment

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

If kerberos is in use for both, would the principal ever be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@twaugh Yes, this koji auth is being internally used by the system for the koji content generator metadata import, the krb5 keytab is expected to be in the buildroot (either via an openshift secret or just drop it in the image). This is to configure exit_koji_promote while running "inner" in order to enable that for Fedora's use case because as of December 12th, ssl cert based auth will be completely disabled in Fedora's koji. (Do note that this getting merged and released by Dec 12th is not a blocker, I can patch our package in the internal Infrastructure dnf repos if needed so there's no real urgency here).

Copy link
Member

Choose a reason for hiding this comment

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

OK, so I prefer koji_use_kerberos and the koji_* keytab and principal parameters, and we just need to check that those parameters are set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@twaugh when you say "need to check" is there something not currently being done in this pull request that you would like to see done in the code or is that just a general comment?

Copy link
Member

Choose a reason for hiding this comment

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

From @lcarva's original comment, should we change this line to something like this?:

if (self.spec.koji_use_kerberos.value and
        self.spec.koji_kerberos_principal.value and
        self.spec.koji_kerberos_keytab.value):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, ok. I follow. Thank you.

self.dj.dock_json_set_arg('exit_plugins', 'koji_promote',
'koji_principal', self.spec.koji_kerberos_principal.value)
self.dj.dock_json_set_arg('exit_plugins', 'koji_promote',
'koji_keytab', self.spec.koji_kerberos_keytab.value)
else:
logger.info("removing koji_promote from request as no kojihub "
"specified")
Expand Down
9 changes: 8 additions & 1 deletion osbs/build/spec.py
View file
Open in desktop
Expand Up @@ -147,6 +147,9 @@ class BuildSpec(object):
kojihub = BuildParam("kojihub", allow_none=True)
koji_certs_secret = BuildParam("koji_certs_secret", allow_none=True)
koji_task_id = BuildParam("koji_task_id", allow_none=True)
koji_use_kerberos = BuildParam("koji_use_kerberos", allow_none=True)
koji_kerberos_principal = BuildParam("koji_kerberos_principal", allow_none=True)
koji_kerberos_keytab = BuildParam("koji_kerberos_keytab", allow_none=True)
image_tag = BuildParam("image_tag")
pulp_secret = BuildParam("pulp_secret", allow_none=True)
pulp_registry = BuildParam("pulp_registry", allow_none=True)
Expand Down Expand Up @@ -197,7 +200,8 @@ def set_params(self, git_uri=None, git_ref=None,
sources_command=None, architecture=None, vendor=None,
build_host=None, authoritative_registry=None, distribution_scope=None,
koji_target=None, kojiroot=None, kojihub=None, koji_certs_secret=None,
koji_task_id=None,
koji_use_kerberos=None, koji_kerberos_keytab=None,
koji_kerberos_principal=None, koji_task_id=None,
source_secret=None, # compatibility name for pulp_secret
pulp_secret=None, pulp_registry=None, pdc_secret=None, pdc_url=None,
smtp_uri=None, nfs_server_path=None,
Expand Down Expand Up @@ -240,6 +244,9 @@ def set_params(self, git_uri=None, git_ref=None,
self.kojiroot.value = kojiroot
self.kojihub.value = kojihub
self.koji_certs_secret.value = koji_certs_secret
self.koji_use_kerberos.value = koji_use_kerberos
self.koji_kerberos_principal.value = koji_kerberos_principal
self.koji_kerberos_keytab.value = koji_kerberos_keytab
self.koji_task_id.value = koji_task_id
self.pulp_secret.value = pulp_secret or source_secret
self.pulp_registry.value = pulp_registry
Expand Down
9 changes: 9 additions & 0 deletions osbs/conf.py
View file
Open in desktop
Expand Up @@ -211,6 +211,15 @@ def get_koji_target(self):
def get_koji_certs_secret(self):
return self._get_value("koji_certs_secret", self.conf_section, "koji_certs_secret")

def get_koji_use_kerberos(self):
return self._get_value("koji_use_kerberos", self.conf_section, "koji_use_kerberos", is_bool_val=True)

def get_koji_kerberos_keytab(self):
return self._get_value("koji_kerberos_keytab", self.conf_section, "koji_kerberos_keytab")

def get_koji_kerberos_principal(self):
return self._get_value("koji_kerberos_principal", self.conf_section, "koji_kerberos_principal")

def get_sources_command(self):
return self._get_value("sources_command", self.conf_section, "sources_command")

Expand Down
51 changes: 51 additions & 0 deletions tests/build/test_build_request.py
View file
Open in desktop
Expand Up @@ -1279,6 +1279,57 @@ def test_render_prod_request_with_koji_secret(self, tmpdir):
koji_certs_secret_name)
assert get_plugin(plugins, 'exit_plugins', 'koji_promote')['args']['koji_ssl_certs'] == mount_path

def test_render_prod_request_with_koji_kerberos(self, tmpdir):
self.create_image_change_trigger_json(str(tmpdir))
build_request = BuildRequest(str(tmpdir))
name_label = "fedora/resultingimage"
push_url = "ssh://{username}git.example.com/git/{component}.git"
koji_task_id = 1234
koji_use_kerberos = True
koji_kerberos_keytab = "FILE:/tmp/fakekeytab"
koji_kerberos_principal = "myprincipal@OSBSDOMAIN.COM"
kwargs = {
'git_uri': TEST_GIT_URI,
'git_ref': TEST_GIT_REF,
'git_branch': TEST_GIT_BRANCH,
'user': "john-foo",
'component': TEST_COMPONENT,
'base_image': 'fedora:latest',
'name_label': name_label,
'registry_uri': "example.com",
'openshift_uri': "http://openshift/",
'builder_openshift_url': "http://openshift/",
'koji_target': "koji-target",
'kojiroot': "http://root/",
'kojihub': "http://hub/",
'sources_command': "make",
'koji_task_id': koji_task_id,
'koji_use_kerberos': koji_use_kerberos,
'koji_kerberos_keytab': koji_kerberos_keytab,
'koji_kerberos_principal': koji_kerberos_principal,
'vendor': "Foo Vendor",
'authoritative_registry': "registry.example.com",
'distribution_scope': "authoritative-source-only",
'registry_api_versions': ['v1'],
'git_push_url': push_url.format(username='', component=TEST_COMPONENT),
'git_push_username': 'example',
}
build_request.set_params(**kwargs)
build_json = build_request.render()

assert build_json["metadata"]["labels"]["koji-task-id"] == str(koji_task_id)

plugins = get_plugins_from_build_json(build_json)
assert get_plugin(plugins, "exit_plugins", "koji_promote")
assert plugin_value_get(plugins, "exit_plugins", "koji_promote",
"args", "kojihub") == kwargs["kojihub"]
assert plugin_value_get(plugins, "exit_plugins", "koji_promote",
"args", "url") == kwargs["openshift_uri"]

assert get_plugin(plugins, 'exit_plugins', 'koji_promote')['args']['koji_principal'] == koji_kerberos_principal
assert get_plugin(plugins, 'exit_plugins', 'koji_promote')['args']['koji_keytab'] == koji_kerberos_keytab


@pytest.mark.parametrize(('base_image', 'is_custom'), [
('fedora', False),
('fedora:latest', False),
Expand Down