Skip to content

Conversation

@NickKeller
Copy link
Contributor

Purpose

Update charts to be compliant with AKS best practices

  • ...

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

@pauldotyu pauldotyu force-pushed the nikelle/storebestpractices branch from a7e8881 to baab047 Compare December 2, 2025 16:52
Copilot AI review requested due to automatic review settings December 2, 2025 16:52
@pauldotyu
Copy link
Contributor

/test-e2e

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

🚀 @pauldotyu Starting E2E tests...

Workflow Run: 19866572631
Environment: e2etest
Location: australiaeast

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Kubernetes manifests to align with AKS (Azure Kubernetes Service) best practices by improving configuration management and adding health probes. The changes move sensitive credentials from plaintext environment variables to Secrets and non-sensitive configuration to ConfigMaps, using envFrom for cleaner configuration injection.

Key Changes:

  • Replaced individual env variables with envFrom references to ConfigMaps and Secrets for RabbitMQ and order-service
  • Added readiness and liveness probes to RabbitMQ container in the quickstart manifest
  • Fixed missing newlines at end of Helm chart template files

Reviewed changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
charts/aks-store-demo/templates/rabbitmq.yaml Added missing newline at end of file
charts/aks-store-demo/templates/order-service.yaml Added missing newline at end of file
charts/aks-store-demo/templates/mongodb.yaml Added missing newline at end of file
aks-store-ingress-quickstart.yaml Introduced ConfigMaps/Secrets for RabbitMQ and order-service configuration, attempted to add health probes to RabbitMQ (contains duplicate probe definitions)
aks-store-all-in-one.yaml Introduced ConfigMaps/Secrets for RabbitMQ and order-service configuration using envFrom pattern

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@pauldotyu
Copy link
Contributor

@NickKeller appreciate you looking into this! I've updated all the YAML for best-practices based on the latest version of Deployment Safeguards in a previous commit before realizing you've made some changes to the Helm chart templates. I've rebased keeping your configmap and secrets changes and running e2e tests now to confirm it'll all still work.

Will resolve Copilot's comments about secrets because I don't think there is any better way around it while keeping deployments simple.

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

@pauldotyu E2E tests completed successfully!

Workflow Run: 19866572631
Duration: ~35 minutes
Status: Passed
Environment: e2etest
Location: australiaeast

All tests passed and resources have been cleaned up.

@pauldotyu pauldotyu merged commit a55c55f into Azure-Samples:main Dec 2, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants