summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
...
* Use RENEWAL_CA_NAME and RA_AGENT_PROFILE constantsFraser Tweedale2019-07-223-10/+10
| | | | | | | | | Replace renewal CA and profile name literals with corresponding symbols from ipalib.constants. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* cainstance: add profile to IPA RA tracking requestFraser Tweedale2019-07-223-0/+4
| | | | | | | | | | | | | Profile-based renewal means we should always explicitly specify the profile in tracking requests that use the dogtag-ipa-ca-renew-agent renewal helper. This includes the IPA RA agent certificate. Update CAInstance.configure_agent_renewal() to add the profile to the tracking request. This also covers the upgrade scenario (because the same method gets invoked). Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* upgrade: fix spurious certmonger re-trackingFraser Tweedale2019-07-221-1/+1
| | | | | | | | | | | The search for the HTTP Certmonger tracking request uses an incorrect parameter ('key-storage'), triggering removal and recreation of tracking requests on every upgrade. Replace 'key-storage' with the correct parameter, 'key-file'. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* upgrade: log missing/misconfigured tracking requestsFraser Tweedale2019-07-221-2/+15
| | | | | | | | | | For better diagnostics during upgrade, log the Certmonger tracking requests that were not found (either because they do not exist, or do not have the expected configuration). Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* upgrade: update KRA tracking requestsFraser Tweedale2019-07-221-3/+12
| | | | | | | | | | | | | | | The upgrade routine checks tracking requests for CA system certificates, IPA RA and HTTP/LDAP/KDC service certificates. If a tracking request matching our expectations is not found, we stop tracking all certificates, then create new tracking requests with the correct configuration. But the KRA was left out. Add checks for KRA certificates, and remove/recreate KRA tracking requests when appropriate. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* upgrade: always add profile to tracking requestsFraser Tweedale2019-07-222-5/+3
| | | | | | | | | | The profile for every Dogtag system cert tracking request is now explicitly specified. So remove the code that handled unspecified profiles. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* dogtaginstance: avoid special cases for Server-CertFraser Tweedale2019-07-223-44/+22
| | | | | | | | | | | | | The Dogtag "Server-Cert cert-pki-ca" certificate is treated specially, with its own track_servercert() method and other special casing. But there is no real need for this - the only (potential) difference is the token name. Account for the token name difference with a lookup method and treat all Dogtag system certs equally w.r.t. tracking request creation and removal. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* dogtag-ipa-ca-renew-agent: always use profile-based renewalFraser Tweedale2019-07-221-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | Update the renewal helper to always request a new certificate ("enrollment request") instead of using "renewal request". The latter is brittle in the face of: - missing certificate record in database - missing original request record in database (pointed to by certificate record) - "mismatched" certificate or request records (there have been many cases of this; it is suspected that request/serial range conflicts, or something similar, may be the cause) The Dogtag tracking request must know what profile to use, except where the certificate uses the default profile ("caServerCert" per 'dogtag-ipa-renew-agent' implementation in Certmonger itself). This part of the puzzle was dealt with in previous commits. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* certmonger: use long options when invoking dogtag-ipa-renew-agentFraser Tweedale2019-07-221-1/+1
| | | | | | | | | | | | To aid reader comprehension, use long options instead of short options when invoking dogtag-ipa-renew-agent. -N -> --force-new -O -> --approval-option Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* upgrade: add profile to Dogtag tracking requestsFraser Tweedale2019-07-223-39/+24
| | | | | | | | | | | | | | | | To use profile-based renewal (rather than "renewal existing cert" renewal which is brittle against database corruption or deleted certificate / request objects), Certmonger tracking requests for Dogtag system certs must record the profile to be used. Update the upgrade method that checks tracking requests to look for the profile. Tracking requests will be recreated if the expected data are not found. The code that actually adds the tracking requests was updated in a previous commit. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* dogtaginstance: add profile to tracking requestsFraser Tweedale2019-07-223-8/+18
| | | | | | | | | | | | | | | | | | | | | | Enabling "fresh" renewals (c.f. "renewal"-based renewals that reference the expired certificate and its associated request object) will improve renewal robustness. To use fresh renewals the tracking request must record the profile to be used. Make dogtaginstance record the profile when creating tracking requests for both CA and KRA. Note that 'Server-Cert cert-pki-ca' and the 'IPA RA' both use profile 'caServerCert', which is the default (according to dogtag-ipa-renew-agent which is part of Certmonger). So we do not need any special handling for those certificates. This commit does not handle upgrade. It will be handled in a subsequent commit. Part of: https://pagure.io/freeipa/issue/7991 Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* Remove posixAccount from service_find search filterRob Crittenden2019-07-191-1/+0
| | | | | | | | | | | | | | This will allow cifs principals to be found. They were suppressed because they include objectclass=posixAccount. This is a bit of a historical anomaly. This was included in the filter from the initial commit (though it was person, not posixAccount). I believe it was a mistake from the beginning but it wasn't noticed because it didn't cause any obvious issues. https://pagure.io/freeipa/issue/8013 Reviewed-By: Alexander Bokovoy <abbra@users.noreply.github.com>
* ci: add --external-ca-profile tests to gatingFraser Tweedale2019-07-171-0/+12
| | | | | | Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* ci: add --external-ca-profile tests to nightlyFraser Tweedale2019-07-175-0/+60
| | | | | | Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Collapse --external-ca-profile tests into single classFraser Tweedale2019-07-171-8/+26
| | | | | | | | | | | | | | To avoid having to spawn new CI hosts for each kind of --external-ca-profile argument we are testing, collapse the three separate test classes into one. Uninstall the half-installed IPA after each section of tests. This change is in response to review comment https://github.com/freeipa/freeipa/pull/2852#pullrequestreview-220442170. Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Add more tests for --external-ca-profile handlingFraser Tweedale2019-07-171-2/+95
| | | | | | | | | | Add tests for remaining untested scenarios of --external-ca-profile handling in ipa-server-install. ipa-ca-install and ipa-cacert-manage remain untested at present. Fixes: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Fix use of incorrect variableFraser Tweedale2019-07-171-1/+1
| | | | | | Part of: https://pagure.io/freeipa/issue/7548 Related: https://pagure.io/freeipa/issue/5608 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* install: fix --external-ca-profile optionFraser Tweedale2019-07-172-6/+0
| | | | | | | | | | | | | | | | | | Commit dd47cfc75a69618f486abefb70f2649ebf8264e7 removed the ability to set pki_req_ext_oid and pki_req_ext_data in the pkispawn config. This results in the --external-ca-profile option never setting the requested values in the CSR (the default V1 template type specifying "SubCA" is always used). Remove relevant fields from both ipaca_default.ini and ipaca_customize.ini. This allows the IPA framework to set the values (i.e. when --external-ca-type=ms-cs and --external-ca-profile=... demand it). It also allows users to override the pki_req_ext_* settings. Part of: https://pagure.io/freeipa/issue/7548 Related: https://pagure.io/freeipa/issue/5608 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* move MSCSTemplate classes to ipalibFraser Tweedale2019-07-178-324/+307
| | | | | | | | | | | | | As we expand the integration tests for external CA functionality, it is helpful (and avoids duplication) to use the MSCSTemplate* classes. These currently live in ipaserver.install.cainstance, but ipatests is no longer permitted to import from ipaserver (see commit 81714976e5e13131654c78eb734746a20237c933). So move these classes to ipalib. Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* certmaprule: add negative test for altSecurityIdentitiesAlexander Bokovoy2019-07-171-0/+29
| | | | | | | | | Try to create a certmap rule that mentiones altSecurityIdentities in its mapping rule but uses IPA domain to apply to. It should fail with ValidationError. Related: https://pagure.io/freeipa/issue/7932 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* certmap rules: altSecurityIdentities should only be used for trusted domainsAlexander Bokovoy2019-07-171-0/+73
| | | | | | | | | | | IPA LDAP has no altSecurityIdentities in use, it only should apply to identities in trusted Active Directory domains. Add checks to enforce proper certmap rule attribution for specific Active Directory domains. Related: https://pagure.io/freeipa/issue/7932 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Create indexes for altSecurityIdentities and ipaCertmapData attributesAlexander Bokovoy2019-07-172-0/+30
| | | | | | | | | | | | | | During an investigation into filter optimisation in 389DS it was discovered that two attributes of the certmap query are unindexed. Due to the nature of LDAP filters, if any member of an OR query is unindexed, the entire OR becomes unindexed. This is then basically a full-table scan, which applies the filter test to the contained members. Fixes: https://pagure.io/freeipa/issue/7932 Fixes: https://pagure.io/freeipa/issue/7933 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Add altSecurityIdentities attribute from MS-WSPP schema definitionAlexander Bokovoy2019-07-172-0/+7
| | | | | | | | | | | | | | | | | | Active Directory schema includes altSecurityIdentities attribute which presents alternative security identities for a bindable object in Active Directory. FreeIPA doesn't currently use this attribute. However, SSSD certmap library may generate searches referencing the attribute if it is specified in the certificate mapping rule. Such search might be considered unindexed in 389-ds. Define altSecurityIdentities attribute to allow specifying indexing rules for it. Fixes: https://pagure.io/freeipa/issue/7932 Related: https://pagure.io/freeipa/issue/7933 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Use stage and phase attempt counters when saving test artifactsAlexander Bokovoy2019-07-171-1/+1
| | | | | | | | | | | | | Azure Pipelines provide counters for running test jobs, these split into System.StageAttempt and System.PhaseAttempt. Use them to make test artifacts unique. For XML test results we don't need to name them differently as they aren't uploaded as artifacts but rather presented in a separate test pane. Reviewed-By: Florence Blanc-Renaud <flo@redhat.com> Reviewed-By: Stanislav Levin <slev@altlinux.org>
* Use any nodejs version instead of forcing a version before nodejs 11Alexander Bokovoy2019-07-174-5/+4
| | | | | | | | | | | Fedora nodejs builds were fixed, we don't need to limit ourselves anymore. Also, make sure python3-pyyaml is installed because pylint in Fedora 31 detects its use in contribs/ Reviewed-By: Florence Blanc-Renaud <flo@redhat.com> Reviewed-By: Stanislav Levin <slev@altlinux.org>
* Fix rpmlint errors for RawhideAlexander Bokovoy2019-07-171-13/+27
| | | | | Reviewed-By: Florence Blanc-Renaud <flo@redhat.com> Reviewed-By: Stanislav Levin <slev@altlinux.org>
* Avoid use of '/tmp' for pip operationsStanislav Levin2019-07-162-6/+18
| | | | | | | | | | | | | | | | | | | | `ipa-run-tests` is not an entry_point script, so pip during an installation of ipatests package checks if the file path is executable. If not - just don't set the executable permission bits. pip's working directory defaults to /tmp/xxx. Thus, if /tmp is mounted with noexec such scripts lose their executable ability after an installation into virtualenv. This was found on Travis + freeipa/freeipa-test-runner:master-latest docker image. Build directory of pip could be changed via env variable PIP_BUILD, for example. Fixes: https://pagure.io/freeipa/issue/8009 Signed-off-by: Stanislav Levin <slev@altlinux.org> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* Make use of Azure Pipeline slicingStanislav Levin2019-07-167-15/+351
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The unit tests execution time within Azure Pipelines(AP) is not balanced. One test job(Base) takes ~13min, while another(XMLRPC) ~28min. Fortunately, AP supports slicing: > An agent job can be used to run a suite of tests in parallel. For example, you can run a large suite of 1000 tests on a single agent. Or, you can use two agents and run 500 tests on each one in parallel. To leverage slicing, the tasks in the job should be smart enough to understand the slice they belong to. >The step that runs the tests in a job needs to know which test slice should be run. The variables System.JobPositionInPhase and System.TotalJobsInPhase can be used for this purpose. Thus, to support this pytest should know how to split the test suite into groups(slices). For this, a new internal pytest plugin was added. About plugin. - Tests within a slice are grouped by test modules because not all of the tests within the module are independent from each other. - Slices are balanced by the number of tests within test module. - To run some module within its own environment there is a dedicated slice option (could help with extremely slow tests) Examples. - To split `test_cmdline` tests into 2 slices and run the first one: ipa-run-tests --slices=2 --slice-num=1 test_cmdline - To split tests into 2 slices, then to move one module out to its own slice and run the second one: ipa-run-tests --slices=2 --slice-dedicated=test_cmdline/test_cli.py \ --slice-num=2 test_cmdline Fixes: https://pagure.io/freeipa/issue/8008 Signed-off-by: Stanislav Levin <slev@altlinux.org> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* Simplify ipa-run-tests scriptStanislav Levin2019-07-169-108/+217
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a sort of rollback to the pre #93c158b05 state with several improvements. For now, the nodeids calculation by ipa-run-tests is not stable, since it depends on current working directory. Nodeids (tests addresses) are utilized by the other plugins, for example. Unfortunately, the `pytest_load_initial_conftests` hook doesn't correctly work with pytest internal paths, since it is called after the calculation of rootdir was performed, for example. Eventually, it's simpler to follow the default convention for Python test discovery. There is at least one drawback of new "old" implementation. The ignore rules don't support globs, because pytest 4.3.0+ has the same facility via `--ignore-glob`: > Add the `--ignore-glob` parameter to exclude test-modules with > Unix shell-style wildcards. Add the collect_ignore_glob for > conftest.py to exclude test-modules with Unix shell-style > wildcards. Upon switching to pytest4 it will be possible to utilize this. Anyway, tests for checking current basic facilities of ipa-run-tests were added. Fixes: https://pagure.io/freeipa/issue/8007 Signed-off-by: Stanislav Levin <slev@altlinux.org> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* upgrade: remove ipaCert and key from /etc/httpd/aliasFlorence Blanc-Renaud2019-07-153-1/+102
| | | | | | | | | | | | | With ipa 4.5+, the RA cert is stored in files in /var/lib/ipa/ra-agent.{key|pem}. The upgrade code handles the move from /etc/httpd/alias to the files but does not remove the private key from /etc/httpd/alias. The fix calls certutil -F -n ipaCert to remove cert and key, instead of -D -n ipaCert which removes only the cert. Fixes: https://pagure.io/freeipa/issue/7329 Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
* ipatests: new test for trust with partially unreachable AD topologySergey Orlov2019-07-152-0/+102
| | | | | | | | | | | | Establishing trust with partially unavailable AD hosts require usage of --server option. The new test checks that both commands trust-add and trust-fetch-domains properly use this option and also that trust-add correctly passes the server value when imlicitly invoking trust-fetch-domains. Relates to: https://pagure.io/freeipa/issue/7895. Reviewed-By: Tibor Dudlak <tdudlak@redhat.com>
* Fix `test_webui.test_selinuxusermap`Stanislav Levin2019-07-156-10/+14
| | | | | | | | | | | | | | | | | | | | | | A previous refactoring of SELinux tests has have a wrong assumption about the user field separator within ipaSELinuxUserMapOrder. That was '$$', but should be just '$'. Actually, '.ldif' and '.update' files are passed through Python template string substitution: > $$ is an escape; it is replaced with a single $. > $identifier names a substitution placeholder matching > a mapping key of "identifier" This means that the text to be substituted on should not be escaped. The wrong ipaSELinuxUserMapOrder previously set will be replaced on upgrade. Fixes: https://pagure.io/freeipa/issue/7996 Fixes: https://pagure.io/freeipa/issue/8005 Signed-off-by: Stanislav Levin <slev@altlinux.org> Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* ipatests/azure: display actual dnf repo URLsFrançois Cami2019-07-081-0/+4
| | | | | | | | | Display which dnf repositories were available at the prepare-build step via metalink. Also display the fastestmirror cache. Signed-off-by: François Cami <fcami@redhat.com> Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* ipatests: mark test_domain_resolution_order as expectedly failingSergey Orlov2019-07-041-0/+4
| | | | | | | SSSD fix have not yet landed in Fedora 29 and below. Relates to https://pagure.io/SSSD/sssd/issue/3957 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* ipatests: add test for sudo with runAsUser and domain resolution order.Sergey Orlov2019-07-041-0/+37
| | | | | | | | | | Running commands with sudo as specific user should succeed when sudo rule has ipasudorunas field defined with value of that user and domain-resolution-order is defined in ipa config. Relates to https://pagure.io/SSSD/sssd/issue/3957 Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
* Use nis-domainname.service on all RH platformsChristian Heimes2019-07-042-17/+1
| | | | | | | | | | RHEL 8 and Fedora >= 29 use "nis-domainname.service" as service name for domainname service. Remove special code in ipaplatform.rhel and for Fedora < 28. Only Fedora 29+ is supported by IPA 4.8. Fixes: https://pagure.io/freeipa/issue/8004 Signed-off-by: Christian Heimes <cheimes@redhat.com> Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* Set git master to 4.9.0Alexander Bokovoy2019-07-031-1/+1
|
* Changing IPA master back to git snapshotsAlexander Bokovoy2019-07-031-3/+3
|
* Become IPA 4.8.0Alexander Bokovoy2019-07-031-5/+5
|
* Make dnf more robust and fasterFrançois Cami2019-07-031-0/+7
| | | | | | | | | | | | | | | | Sometimes the prepare-build step of azure pipelines fails with download errors: "configure: error: Package requirements (nspr) were not met:" This can be due to fastestmirror not being used to check mirror availability and sometimes speed. Combined with a too-low default number of retries, and a high timeout this can lead to download failures that could be avoided. Activate fastestmirror, add more download workers, and tune timeout/retries to make dnf more reliable. Fixes: https://pagure.io/freeipa/issue/7999 Signed-off-by: François Cami <fcami@redhat.com> Reviewed-By: Christian Heimes <cheimes@redhat.com>
* prci: fix nightly_master test definitionsArmando Neto2019-07-031-3/+3
| | | | | Signed-off-by: Armando Neto <abiagion@redhat.com> Reviewed-By: Christian Heimes <cheimes@redhat.com>
* Use system-wide crypto policy for TLS ciphersChristian Heimes2019-07-023-8/+8
| | | | | | | | | | IPA now uses the system-wide crypto policy for TLS ciphers on RHEL. It's also now possible to keep the default policy by setting TLS_HIGH_CIPHERS to None. Fixes: https://pagure.io/freeipa/issue/7998 Signed-off-by: Christian Heimes <cheimes@redhat.com> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* prci: bump ci-master-f30 templateArmando Neto2019-07-023-3/+3
| | | | | | | No major changes, dependencies updated. Signed-off-by: Armando Neto <abiagion@redhat.com> Reviewed-By: Sergey Orlov <sorlov@redhat.com>
* ipatests: fix ipatests/test_xmlrpc/test_dns_plugin.pyFlorence Blanc-Renaud2019-07-021-1/+1
| | | | | | | | The test is calling dnsrecord-mod --ttl and should expect a unicode value in order to be python2/python3 compatible. Related: https://pagure.io/freeipa/issue/7982 Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* translations: update from Zanata Spanish and Ukrainian translationsAlexander Bokovoy2019-07-022-3/+2570
| | | | Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* Add test_smb to night Fedora 30 test suiteRob Crittenden2019-07-021-0/+12
| | | | | | | This exercises the removal of 3DES and RC4 via Samba. Reviewed-By: Robbie Harwood <rharwood@redhat.com> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* Remove DES3 and RC4 enctypes from KerberosRob Crittenden2019-07-021-4/+0
| | | | | | | These are already marked as deprecated by the KDC. Reviewed-By: Robbie Harwood <rharwood@redhat.com> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* Don't configure disabled krb5 enctypes in FIPS modeRob Crittenden2019-07-022-9/+10
| | | | | | | | | | | | | | | | | The only permitted ciphers are the AES family (called aes, which is the combination of: aes256-cts-hmac-sha1-96, aes128-cts-hmac-sha1-96, aes256-cts-hmac-sha384-192, and aes128-cts-hmac-sha256-128). DES, RC4, and Camellia are not permitted in FIPS mode. While 3DES is permitted, the KDF used for it in krb5 is not, and Microsoft doesn't implement 3DES anyway. This is only applied on new installations because we don't allow converting a non-FIPS install into a FIPS one. Reviewed-By: Robbie Harwood <rharwood@redhat.com> Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
* Use only TLS 1.2 by defaultChristian Heimes2019-07-015-22/+32
| | | | | | | | | | | | | | | | | TLS 1.3 is causing some trouble with client cert authentication. Conditional client cert authentication requires post-handshake authentication extension on TLS 1.3. The new feature is not fully implemented yet. TLS 1.0 and 1.1 are no longer state of the art and now disabled by default. TLS 1.2 works everywhere and supports PFS. Related: https://pagure.io/freeipa/issue/7667 Signed-off-by: Christian Heimes <cheimes@redhat.com> Reviewed-By: Rob Crittenden <rcritten@redhat.com>
* For Fedora and RHEL use system-wide crypto policy for mod_sslRob Crittenden2019-07-014-4/+19
| | | | | | | | | | | | | | Drop the SSLProtocol directive for Fedora and RHEL systems. mod_ssl will use crypto policies for the set of protocols. For Debian systems configure a similar set of protocols for what was previously configured, but do it in a different way. Rather than iterating the allowed protocols just include the ones not allowed. Fixes: https://pagure.io/freeipa/issue/7667 Signed-off-by: Rob Crittenden <rcritten@redhat.com> Reviewed-By: Rob Crittenden <rcritten@redhat.com>