fix: new fields and fixes for Terraform, Cargo, Nuget, Helm, Ansible #1257
Conversation
…nsible Signed-off-by: richard.le-terrier <[email protected]>
github-actions
bot
removed
the
safe to test
Signed-off-by: richard.le-terrier <[email protected]>
github-actions
bot
removed
the
safe to test
|
Work in progress to adapt unit tests. |
|
please check CI failures |
|
Please update description and also provide appropriate labels |
| type HelmRemoteRepositoryParams struct { | ||
| RemoteRepositoryBaseParams | ||
| ChartsBaseUrl string `json:"chartsBaseUrl,omitempty"` | ||
| ChartsBaseUrl string `json:"chartsBaseUrl"` // do not set omitempty to be able to empty the value when updating |
There was a problem hiding this comment.
if let's say this used for updating, are we making sure any empty value sent to the server is not updated in the database?
There was a problem hiding this comment.
I'm not sure about the implementation in the Artifactory API since I don't have the source for it. But in the client updating local and remote repositories config will result in a call to the method https://github.com/jfrog/jfrog-client-go/blob/master/artifactory/services/repository.go#L28 that will marshal the struct (remove the empty values from the body) and send a POST request. Maybe the POST is implemented like a PATCH on the server and ignore missing fields.
There was a problem hiding this comment.
Also what strange in that performRequest function is that the update will run a POST while the create will do a PUT.
| setCacheVirtualRepositoryParams(&tvp.CommonCacheVirtualRepositoryParams, true) | ||
|
|
||
| err = testsUpdateVirtualRepositoryService.Terraform(tvp) | ||
| assert.NoError(t, err, "Failed to update "+repoKey) |
There was a problem hiding this comment.
isn't this assertion valid? Is it possible to provide proper comment in the codebase why it was decided to be removed?
There was a problem hiding this comment.
Removed this two lines because there are the same assertions at line 680.
|
|
||
| err = testsUpdateVirtualRepositoryService.Terraform(tvp) | ||
| assert.NoError(t, err, "Failed to update "+repoKey) | ||
| validateRepoConfig(t, repoKey, tvp) |
github-actions
bot
removed
the
safe to test
Signed-off-by: richard.le-terrier <[email protected]>
github-actions
bot
removed
the
safe to test
richardlt
changed the title
4 participants
This PR add some missing fields on local and remote repository structs. The Ansible implmentation was also added.
Another changes that is introduced here is removing some omitempty tags. Without removing this tags it seems impossible to empty some fields when updating the repositories config.