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

Treat values as simple/array based on local lens #65

Merged
merged 1 commit into from
Feb 15, 2024
Merged

Treat values as simple/array based on local lens #65

merged 1 commit into from
Feb 15, 2024

Conversation

tedgarb
Copy link
Contributor

@tedgarb tedgarb commented Apr 15, 2021

This code previously used an explicitly defined list of labels that were
treated as arrays and not simple values. This broke the sshd_config
provider for users with a version of the sshd lens from augeas 1.9.0 or
newer. To avoid breaking support for older users, and to be
forward-compatible with new versions of augeas that are aware of
additional array values, this change determines if a label is an array
or simple value by testing what the underlying augeas provider returns

Addresses #64

@coveralls
Copy link

Coverage Status

Coverage decreased (-66.7%) to 30.279% when pulling 722ed82 on tedgarb:master into a0f3728 on hercules-team:master.

@tedgarb
Copy link
Contributor Author

tedgarb commented Apr 16, 2021

It looks like there are total testing failures here that might be tied not to any specific change I made, but a change in rspec itself. The last successful testing log shows that rspec in the 3.9.x series was used, but this test suite used the 3.10.x series. Please let me know how you want me to proceed here. I have verified this code in actual production use in my puppet environment, so I can at least attest it is not completely broken, even if I cannot pass the test suite here.

@tedgarb
Copy link
Contributor Author

tedgarb commented Jan 19, 2022

I am a bit disappointed to see #68 merged, which only addresses the proximal issue of HostKeyAlgorithms, instead of this change, which would fix both that and any future issues of this type, while allowing a greater range of augeas providers. Could this PR please be reconsidered?

@vox-pupuli-tasks
Copy link

Dear @tedgarb, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@vox-pupuli-tasks
Copy link

Dear @tedgarb, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@tedgarb
Copy link
Contributor Author

tedgarb commented Nov 10, 2022

Sorry for the long silence here... Rebased this and as I hoped the CI tests succeeded now, I still think this is a somewhat more elegant solution to #64 that provides better forward/back compatibility, and politely request this be considered as a more general solution.

@tedgarb
Copy link
Contributor Author

tedgarb commented Jan 4, 2023

Hi, wanted to check on whether there is anything I could do to get this merged?

Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

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

I guess it works but I can't find where parsed_as? is defined. I see it in the changelog here but can't find the code: https://github.com/voxpupuli/puppet-augeasproviders/blob/0c1f9f6a60b29c14633d8820d827aacf568ed5dd/CHANGELOG.md?plain=1#L56

@tedgarb
Copy link
Contributor Author

tedgarb commented Jan 12, 2023

@tedgarb
Copy link
Contributor Author

tedgarb commented Apr 13, 2023

Wanted to check in on this

@tedgarb
Copy link
Contributor Author

tedgarb commented Dec 19, 2023

Checking in on this

@tedgarb
Copy link
Contributor Author

tedgarb commented Feb 14, 2024

Wanted to query this again and ask if there is anything I can do to move it along. There is an errant "tests fail" label on this from a previous issue with the tests, but the tests have been passing for a while.

@zilchms
Copy link
Contributor

zilchms commented Feb 15, 2024

Hey @tedgarb,
sorry for the long wait. I am not "the" maintainer of this module, but I can help you get this merged.
Can you rebase this once on the current master? If everything passes the CI then, I will merge this.
Thanks for your contribution and your seemingly endless patience with us :)

This code previously used an explicitly defined list of labels that were
treated as arrays and not simple values. This broke the sshd_config
provider for users with a version of the sshd lens from augeas 1.9.0 or
newer. To avoid breaking support for older users, and to be
forward-compatible with new versions of augeas that are aware of
additional array values, this change determines if a label is an array
or simple value by testing what the underlying augeas provider returns
@tedgarb
Copy link
Contributor Author

tedgarb commented Feb 15, 2024

Got a little twisted trying to rebase before first coffee, but it's there now, should be good

@zilchms zilchms merged commit 51f3b88 into voxpupuli:master Feb 15, 2024
34 checks passed
@zilchms
Copy link
Contributor

zilchms commented Feb 15, 2024

Thx for your contribution and patiance! :)

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