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

Secret replication #1775

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

akhilmhdh
Copy link
Member

Description 📣

  1. Implemented secret replication feature for import
  2. Resync feature to do manual resync

Type ✨

  • Bug fix
  • New feature
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@akhilmhdh akhilmhdh added the 🚀 feature request New feature or request label May 2, 2024
@akhilmhdh akhilmhdh requested a review from maidul98 May 2, 2024 15:58
@akhilmhdh akhilmhdh self-assigned this May 2, 2024
@sheensantoscapadngan
Copy link
Member

If I declare a personal secret in the source of the import, the replicated secret seems to disappear

image image

If I remove the personal secret, the replicated secret still does not show up

But after triggering a manual resync, the shared secret then shows up
image

@sheensantoscapadngan
Copy link
Member

sheensantoscapadngan commented May 29, 2024

what do you think about adding an icon in the replicated secret entry row which informs the user that it's a replicated secret? Confusion might arise if they try to make updates to replicated secrets because the current behavior overwrites changes completely with the actual values from the source of the replicated import

image

@sheensantoscapadngan
Copy link
Member

sheensantoscapadngan commented May 29, 2024

Dev:
image

Staging:
image

Prod:
image

Updating the import order does not cause changes in the replicated value. is this intentional? this gets resolved if I do a manual sync of the replicated import. But the sync does not respect import order so it just overwrites the value based on which import I manually synced
image

@akhilmhdh
Copy link
Member Author

Order doesn't matter for replicated secret @sheensantoscapadngan

Copy link
Member

@sheensantoscapadngan sheensantoscapadngan left a comment

Choose a reason for hiding this comment

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

what do you think about adding a warning modal that pops up when they try to edit a replicated secret?

@sheensantoscapadngan
Copy link
Member

I think the icon for replicated secrets could be misleading. it's like a "multi-select" icon. not sure but maybe we could just do an information icon tooltip?
image

@akhilmhdh
Copy link
Member Author

akhilmhdh commented May 29, 2024

I think the icon for replicated secrets could be misleading. it's like a "multi-select" icon. not sure but maybe we could just do an information icon tooltip? image

Its a clone icon from fontawesome icon. Info icon doesn't make sense to the whole dashboard. Also kindly keep comments in review comment box. Its hard to reply here.

@akhilmhdh
Copy link
Member Author

what do you think about adding a warning modal that pops up when they try to edit a replicated secret?

The replicated secret is treated as normal to an extend. Only difference is if gets replicated when source changes.

Copy link
Member

@sheensantoscapadngan sheensantoscapadngan left a comment

Choose a reason for hiding this comment

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

lgtm!

@akhilmhdh akhilmhdh marked this pull request as ready for review May 29, 2024 15:49
Copy link
Collaborator

@maidul98 maidul98 left a comment

Choose a reason for hiding this comment

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

Left comments in slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants