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

Update method for checking endpoint protocol #769

Merged
merged 4 commits into from May 4, 2023

Conversation

gnuoy
Copy link
Contributor

@gnuoy gnuoy commented Apr 19, 2023

The https method is used to check if an endpoint is expected to be http or https. One of the checks it performs is to examine the the certificates relation. If the relation is present then it looks for the existance of a CA. However the OpenStack charms do not switch to https until a certificate is provided via the certificates relation. This means there can be a disconnect if the certificate provider has provided a CA but has not yet provided the unit specific certificates. If this happens then the payload will still be using http but the https method will return True.

This patch updates the https method to return False if an unfilled certificate request exists.

https://bugs.launchpad.net/charm-keystone/+bug/2015103

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.
@gnuoy gnuoy marked this pull request as ready for review April 19, 2023 14:06
Copy link
Contributor

@coreycb coreycb left a comment

Choose a reason for hiding this comment

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

LGTM. Could you add the bug reference to the commit message? 2015103

coreycb
coreycb previously approved these changes May 2, 2023
wolsen
wolsen previously approved these changes May 2, 2023
Copy link
Collaborator

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Unfortunately, it looks like a few unit tests are breaking due to the change; it looks like the tests need some additional mocks as the method-under-test eventually calls out to juju primatives.

e.g.

                                                                                                                                                                                                                              
======================================================================                                                                                                                                                        
ERROR: tests.contrib.hahelpers.test_cluster_utils.ClusterUtilsTests.test_https_cert_key_incomplete_identity_relation                                                                                                          
----------------------------------------------------------------------                                                                                                                                                        
testtools.testresult.real._StringException: Traceback (most recent call last):                                                                                                                                                
  File "/home/ubuntu/git/github.com/juju/charm-helpers/charmhelpers/core/hookenv.py", line 1180, in inner_translate_exc2                                                                                                      
    return f(*args, **kwargs)                                                                                                                                                                                                 
  File "/home/ubuntu/git/github.com/juju/charm-helpers/charmhelpers/core/hookenv.py", line 1374, in network_get_primary_address                                                                                               
    response = subprocess.check_output(                                                                                                                                                                                       
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output                                                                                                                                                          
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,                                                                                                                                                          
  File "/usr/lib/python3.8/subprocess.py", line 493, in run                                                                                                                                                                   
    with Popen(*popenargs, **kwargs) as process:                                                                                                                                                                              
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__                                                                                                                                                              
    self._execute_child(args, executable, preexec_fn, close_fds,                                                                                                                                                              
  File "/usr/lib/python3.8/subprocess.py", line 1704, in _execute_child                                                                                                                                                       
    raise child_exception_type(errno_num, err_msg, err_filename)                                                                                                                                                              
FileNotFoundError: [Errno 2] No such file or directory: 'network-get' 

Unfortunately, the github actions are currently broken due to: #774 so you'll need to run them manually (on say focal), until we get this fixed.

@gnuoy gnuoy dismissed stale reviews from wolsen and coreycb via 53677e3 May 3, 2023 09:26
Copy link
Collaborator

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Please see inline test comment; I think a boolean assert should be flipped.

'key', # relation_get('ssl_key')
'ca_cert', # relation_get('ca_cert')
]
self.assertTrue(cluster_utils.https())
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be assertFalse as the request for local unit is {} and that it will return False from https().

@ajkavanagh
Copy link
Collaborator

I've manually verified that the tox pep8 and py3 targets run on focal and jammy (actually py310 due to nose2 requirement). This is in lieu of the fix to the CI runners.

@ajkavanagh ajkavanagh merged commit 6064a34 into juju:master May 4, 2023
0 of 5 checks passed
freyes pushed a commit to freyes/charm-helpers that referenced this pull request May 6, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)
ajkavanagh pushed a commit that referenced this pull request May 10, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request May 26, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
coreycb pushed a commit to coreycb/charm-helpers that referenced this pull request Jun 7, 2023
Update method for checking endpoint protocol

The `https` method is used to check if an endpoint is expected to
be http or https. One of the checks it performs is to examine the
the certificates relation. If the relation is present then it looks
for the existance of a CA. However the OpenStack charms do not
switch to https until a certificate is provided via the certificates
relation. This means there can be a disconnect if the
certificate provider has provided a CA but has not yet provided
the unit specific certificates. If this happens then the payload
will still be using http but the `https` method will return True.

This patch updates the `https` method to return False if an unfilled
certificate request exists.

(cherry picked from commit 6064a34)

Co-authored-by: Liam Young <liam.young@canonical.com>
(cherry picked from commit ed01437)
freyes added a commit that referenced this pull request Jun 8, 2023
[stable/zed] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
[stable/yoga] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
[stable/xena] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
…otocol

[stable/wallaby] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
…rotocol

[stable/victoria] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
…tocol

[stable/ussuri] Update method for checking endpoint protocol (#769) (#775)
freyes added a commit that referenced this pull request Jun 8, 2023
…ocol

[stable/train] Update method for checking endpoint protocol (#769) (#775)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants