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

Fixes #36667 - Hammer host update should fail non-existent LCE #10691

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

lfu
Copy link
Member

@lfu lfu commented Aug 10, 2023

What are the changes introduced in this pull request?

Fixed a bug with hammer host update when --lifecycle-environment-id or --content-view-id is missing.

Considerations taken when implementing this change?

What are the testing steps for this pull request?

  1. Have a host registered to the Satellite
  2. Run hammer host update --id <host_id> --lifecycle-environment-id 999999

Before
Host updated.

After

$ hammer host update --id 3 --lifecycle-environment-id 999999
[ERROR 2023-08-09T22:00:25 Exception] Content view must be specified
$ hammer host update --id 3 --content-view-id 1111
[ERROR 2023-08-09T22:01:34 Exception] Lifecycle environment must be specified
$ hammer host update --id 3 --lifecycle-environment-id 999999 --content-view-id 888
[ERROR 2023-08-09T21:06:19 Exception] Validation failed: Content view environments must have both a content view and an environment
$ hammer -d host update --id 3 --lifecycle-environment-id 999999 --content-view-id 1
[ERROR 2023-08-09T21:58:09 Exception] Validation failed: Content view environments must have both a content view and an environment, Default_Organization_View could not be promoted to  because the conte     nt view and the environment are not in the same organization!

@theforeman-bot
Copy link

Issues: #36667

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Code looks fine, giving it a test

@chris1984 chris1984 self-assigned this Aug 10, 2023
@chris1984
Copy link
Member

@lfu

Looks like rubocop is not happy:

[2023-08-10T14:50:04.714Z] Offenses:

[2023-08-10T14:50:04.714Z] 

[2023-08-10T14:50:04.714Z] /home/jenkins/workspace/katello-pr-test/test/controllers/api/v2/hosts_controller_test.rb:92:36: C: Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.

[2023-08-10T14:50:04.714Z]         :content_view_id => @cv2.id,

[2023-08-10T14:50:04.714Z]                                    ^

[2023-08-10T14:50:04.714Z] 

[2023-08-10T14:50:04.714Z] 1940 files inspected, 1 offense detected

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

ACK works great:
Before:

[root@centos8-stream-katello-nightly ~]# hammer host update --id 1 --lifecycle-environment-id 999999
Host updated.

After:
```
[root@centos8-stream-katello-nightly ~]# hammer host update --id 1 --lifecycle-environment-id 999999
Could not update the host:
  Content view must be specified
```

@lfu lfu merged commit 05b73f7 into Katello:master Aug 16, 2023
5 checks passed
@lfu lfu deleted the cve_validate_36667 branch November 16, 2023 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants