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

Fix for #41858 #52168 #52161

Closed
wants to merge 106 commits into from
Closed

Fix for #41858 #52168 #52161

wants to merge 106 commits into from

Conversation

t0fik
Copy link
Contributor

@t0fik t0fik commented Mar 13, 2019

  1. Fix x509.sign_remote_certificate not working after upgrade to 2019.2.0 #51869 issue
    Removes kwargs stringification before pushing them to publish.publish.
    Stringification was causing adding redundant unicode identifier to dictionary keys after using salt.utils.yaml.safe_loader.
    In 2018.3 yaml loader was cleaning redundant unicode identifiers.
    ex: u"u'signing_policy'" instead u'signing_policy'
    Resolved by Fix issue #51869 and add cert signing test #52381

  2. Fix x509.certificate_managed can write an error message into the cert file instead of failing #41858 and x509.certificate_managed can write an error message as the certificate #52168 issue
    Exceptions raised in get_pem_entry was suppressed by salt.utils.files.set_umask. It was leading to writing incorrect values (ex. error message) to destination pem file.

  3. Fixed compound match execution in sign_remote_certificate
    match.compound was called on local (ca_server) not on minion requesting certificate sign

@t0fik t0fik changed the title Removed kwargs stringification Removed kwargs stringification. Fix for #51896 Mar 13, 2019
@t0fik t0fik changed the title Removed kwargs stringification. Fix for #51896 Fix for #51896 and #41858 Mar 14, 2019
@t0fik t0fik changed the title Fix for #51896 and #41858 Fix for #51896 #41858 #52168 Mar 14, 2019
@dwoz dwoz self-requested a review April 11, 2019 22:56
Copy link
Contributor

@dwoz dwoz left a comment

Choose a reason for hiding this comment

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

@t0fik This needs some regression tests.

@dwoz dwoz added the v2019.2.1 unsupported version label Apr 11, 2019
@dwoz
Copy link
Contributor

dwoz commented Apr 11, 2019

I've added the 2019.2.1 label so it can get into that release provided regression tests are written.

@t0fik
Copy link
Contributor Author

t0fik commented Apr 12, 2019

@dwoz Extending tests for x509 state would be ok? Or you want new tests for x509 module?

@dwoz
Copy link
Contributor

dwoz commented Apr 16, 2019

@t0fik I'm guessing new tests so we don't mix testing concerns. However, whatever you think gives best coverage of the change is fine.

@t0fik
Copy link
Contributor Author

t0fik commented Apr 17, 2019

@dwoz I should have time to write tests at Easters.

cncshaggy and others added 9 commits May 7, 2019 05:21
caught a typo
Separation of concerns, and reduces the amount of mocking that needs to be
done in order to isolate bug.
The mock values were obtained by running an acutal query, truncating
the list of returned entries and stripping unnecessary keys from the
results.

The test case runs well with python 2 but causes a traceback with
python 3.
With python 3, salt-cloud tracebacks in salt/cloud/clouds/ec2.py's
get_imageid when searching for an image name not starting with "ami-".
Python 3 removed the cmp parameter in favor of key.
…o another location. This fix ensures that when fsroot is referenced we are using the real path and not the symlink path.
@t0fik t0fik changed the title Fix for #51896 #41858 #52168 Fix for #41858 #52168 May 16, 2019
waynew and others added 18 commits July 3, 2019 17:24
…che-exception

Use encoding when caching pillar data
…user_exists_fixes

[2019.2] More fixes to mysql module
increase sleep time between kitchen create failures to account for api limits
[2019.2] Update CODEOWNERS github file
Update release versions for 2019.2 branch
@sagetherage sagetherage removed the v2019.2.1 unsupported version label Mar 11, 2020
@sagetherage
Copy link
Contributor

@t0fik this will need to get rebased with master @dwoz can you help with tests?

@t0fik t0fik requested a review from a team as a code owner March 13, 2020 07:47
@ghost ghost requested a review from twangboy March 13, 2020 07:47
@t0fik t0fik mentioned this pull request Mar 13, 2020
@t0fik
Copy link
Contributor Author

t0fik commented Mar 13, 2020

@sagetherage I've messed up on rebasing, so I've created new clean and shiny PR to develop #56367

@t0fik t0fik closed this Mar 13, 2020
@t0fik t0fik mentioned this pull request Mar 13, 2020
@t0fik
Copy link
Contributor Author

t0fik commented Mar 13, 2020

I've also created PR for master branch #56372

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Reviewers-Assigned
Projects
None yet
Development

Successfully merging this pull request may close these issues.