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

Updating an existing artifact store configuration with a new key silently fails (if in use?) #10735

Open
chrisb2244 opened this issue Aug 23, 2022 · 4 comments

Comments

@chrisb2244
Copy link

Issue Type
  • Bug Report
Summary

I added a new property to an Artifact Plugin (for the Store Config) but when I then added a value for that property to an existing store, it was ignored and silently ignored.
Adding it directly to the config XML works.

Removing then re-adding the store is impossible because GoCD prevents removing stores that are referenced by pipelines (including pipelines from config repositories).

Basic environment details
  • Go Version: 22.2.0
  • JAVA Version: 17.0.4
  • OS: Windows 10
  • Browser vendor and version (if relevant): Chrome 104.0.5112.101 (64-bit)
Steps to Reproduce

Will try setup a reproduction against the GoCD Docker plugin, otherwise, can provide the git hashes of the before/after from my current plugin once I release it next month.
(note to self, this would be something like a4aa31c and f1129d0, although those hashes are useless to this issue at present).

Reproduction probably requires two version of a plugin with different artifact store entries (I only observed the case where a new property is added, untried with removing properties) and then I'd imagine a pipeline using the config in the version 1 of the plugin.

Replace with the new version (2), then go to Admin > Artifact Stores > Edit (for chosen store), fill in the new value (appears correctly in the configuration dialog).

Verify by any of (I think):

  • Reopen the dialog, the field is blank/default valued
  • Browse the config XML, the property doesn't exist
  • Run a pipeline that uses that property, see that it gets an empty/null/default-ish value and then behaves unexpectedly (obviously this is how I found it... :doh:)
Expected Results

Ideally, the property should be added.
Alternatively, the configuration should warn that it could not be / was not updated, and that the user should manually adjust the config XML.

Actual Results

Looks good, silently does nothing, returns whatever the behaviour of deserializing a non-existent property is (presumably depends on the plugin deserialization method).

Any other info

Can be worked around by manually editing the server XML config.

@stale
Copy link

stale bot commented Nov 23, 2022

This issue has been automatically marked as stale because it has not had activity in the last 90 days.
If you can still reproduce this error on the master branch using local development environment or on the latest GoCD Release, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@stale stale bot added the stale label Nov 23, 2022
@stale stale bot closed this as completed Dec 21, 2022
@chadlwilson chadlwilson added no stalebot Don't mark this stale. and removed stale labels Dec 21, 2022
@chadlwilson chadlwilson reopened this Dec 21, 2022
@chadlwilson
Copy link
Member

Did you ever get around to finding a way to reproduce this? I'm not sure I quite follow how this problem is created here, and how much depends on the design of the plugin, optional vs required properties etc.

  • Was the new property optional or required in the metadata?
  • Did the correct data get sent from the UI to the server?
  • Did the plugin get invoked with a message to validate the store config?
  • Did the server send the right details to the plugin to validate it?

@chadlwilson chadlwilson removed the no stalebot Don't mark this stale. label Mar 9, 2023
@chrisb2244
Copy link
Author

So I think this is a fairly niche issue, and perhaps unlikely to affect many others (and it is a one-time problem if you can fix it via the workaround listed in the OP as far as I remember), but to answer your questions for potential future readers:

  • In my case, the property was marked as required (ng-required="true", and the validation threw errors if it was empty). I don't think this is critical to the issue (but probably for optional properties, developers would add better handling for a missing property).
  • I'm unsure, but as I remember, the validation triggered correctly and complained when the value added to the UI dialog didn't match the validation rules that were set (URL format + not empty in my case)

@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been automatically marked as stale because it has not had activity in the last 90 days.
If you can still reproduce this error on the master branch using local development environment or on the latest GoCD Release, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@stale stale bot added the stale label Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants