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.

Sign up for GitHub

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

Jump to bottom

New Drop - Arc K8s Secret Store extension #223

Merged
merged 36 commits into from
Feb 10, 2025
Merged

New Drop - Arc K8s Secret Store extension #223

merged 36 commits into from
Feb 10, 2025

Conversation

zaidmohd
Copy link
Contributor

@zaidmohd zaidmohd commented Feb 4, 2025

New Drop - Arc K8s Secret Store extension. This includes

  • Drop json
  • Bicep and script automation

@zaidmohd zaidmohd requested a review from Copilot February 4, 2025 22:05
Copilot
Copilot AI reviewed Feb 4, 2025
View reviewed changes

Choose a reason for hiding this comment

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

Copilot reviewed 11 out of 18 changed files in this pull request and generated no comments.

Files not reviewed (7)
  • drops/arc_k8s_secret_store_extension.json: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/main.bicep: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/main.bicepparam: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/modules/k3s.bicep: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/modules/mgmtArtifacts.bicep: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/scripts/k3sWithSSE.sh: Language not supported
  • script_automation/arc_k8s_secret_store_extension/artifacts/Bicep/scripts/welcomeK3s.sh: Language not supported
Comments suppressed due to low confidence (1)

script_automation/arc_k8s_secret_store_extension/README.md:38

  • The word 'retrive' is misspelled. It should be 'retrieve'.
To retrive the SSH public key after it's been created, depending on your environment, use one of the below methods:
@zaidmohd zaidmohd requested a review from kianaharris4 February 4, 2025 22:05
@kianaharris4 kianaharris4 requested a review from likamrat February 4, 2025 22:09
kianaharris4
kianaharris4 requested changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@kianaharris4 kianaharris4 left a comment

Choose a reason for hiding this comment

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

Left a few comments - @likamrat will also review!

likamrat
likamrat reviewed Feb 5, 2025
View reviewed changes
rebeccaleigh
rebeccaleigh suggested changes Feb 7, 2025
View reviewed changes
Copy link

@rebeccaleigh rebeccaleigh left a comment

Choose a reason for hiding this comment

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

Zaid - overall looks great, just some suggestions.

script_automation/arc_k8s_secret_store_extension/README.md

## Getting Started

### Run the automation
Copy link

Choose a reason for hiding this comment

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

High level question- why is everything automated via the script for this drop but for other similar drops like ACSA (https://github.com/Azure/arc_jumpstart_drops/tree/main/script_automation/arc_edge_volumes_cloudsync_ubuntu_k3_single), only some configuration is automated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our goal is to ensure all the necessary infrastructure is in place so that users never encounter failures and can easily experiment with the feature. Providing step-by-step instructions would be akin to documentation. Adding @likamrat for thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zaidmohd is correct. Also, unlike this Drop that was developed by a core Jumpstart maintainer, the ACSA one was contributed by ACSA team. When we develop in-house, we follow our design principles mentioned by @zaidmohd.

Copy link

Choose a reason for hiding this comment

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

Sounds great - thank you for explaining

@kianaharris4 kianaharris4 merged commit 652d7a5 into Azure:canary Feb 10, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebeccaleigh rebeccaleigh rebeccaleigh approved these changes

Copilot code review Copilot Copilot left review comments

@kianaharris4 kianaharris4 kianaharris4 approved these changes

@likamrat likamrat likamrat approved these changes

Assignees
No one assigned
Labels
review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zaidmohd @kianaharris4 @likamrat @rebeccaleigh