Enable modify button for app repositories #1706
Conversation
dashboard/src/actions/repos.ts
Outdated
| @@ -248,6 +256,8 @@ export const updateRepo = ( | |||
| registrySecrets, | |||
| ); | |||
| dispatch(repoUpdated(data.appRepository)); | |||
| // Re-fetch secrets in case there are updates | |||
| dispatch(fetchRepoSecrets(namespace)); | |||
There was a problem hiding this comment.
So given the different secret associated with an AppRepository - helm repository secrets and docker registry image pull secrets, this could be confusing? Perhaps a more explicit comment:
// Re-fetch the helm repo secret that could have been modified with the updated headers
// so that if the user chooses to edit the app repo again, they will see the current value.
if I have understood correctly? Also, don't we expect only ever one secret for the helm repository access? If so, why aren't we encoding that anywhere? Finally, would the code be simpler if the AppRepository actually stored in a field the name of the helm repository secret?
There was a problem hiding this comment.
we can request just one secret, yes
dashboard/src/actions/repos.ts
Outdated
| // Re-fetch the helm repo secret that could have been modified with the updated headers | ||
| // so that if the user chooses to edit the app repo again, they will see the current value. | ||
| if (data.appRepository.spec?.auth) { | ||
| const secretName = data.appRepository.spec.auth.header.secretKeyRef.name; |
There was a problem hiding this comment.
Thanks for the update. Unless I'm missing something, if an app repository has a spec.auth.customCA but not a header, this will miss loading the secret? The underlying issue is that we (kubeapps) only ever write a single secret with potentially both pieces of data, but the model allows a user to manually create an AppRepository with two separate secrets, I think? That seems like something we can't change easily (without migrating people's AppRepository resources), so we actually need to check both secretKeyRef's which will normally be the same, and load one or both (just use a set and iterate - 2 extra lines, I think).
There was a problem hiding this comment.
Good catch, yes, that's correct.
Description of the change
Requires #1705
I have enabled the modify button which was mostly ready unless two minor things:
Since the reducer for app repos didn't exist, I have covered part of that file with unit tests.
Applicable issues