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

feat(ingress): add path as property configurable #196

Merged
merged 5 commits into from
Jun 4, 2024
Merged

feat(ingress): add path as property configurable #196

merged 5 commits into from
Jun 4, 2024

Conversation

jasondiazg
Copy link
Contributor

@jasondiazg jasondiazg commented May 17, 2024 •
edited
Loading

Description of the change

These changes convert ingress.path as a configurable property. The change is needed for the deployments in a specific path instead of the base domain.

Existing or Associated Issue(s)

Closes #195

Additional Information

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
  • JSON Schema generated.
  • List tests pass for Chart using the Chart Testing tool and the ct lint command.

@jasondiazg jasondiazg requested a review from a team as a code owner May 17, 2024 22:03
@sabre1041
Copy link
Contributor

@jasondiazg would you be able to sign the DCO?

@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch from f10cdee to 487b272 Compare May 20, 2024 19:16
@jasondiazg
Copy link
Contributor Author

@jasondiazg would you be able to sign the DCO?

Sorry @sabre1041 I did it now.

@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch 3 times, most recently from a7a8790 to 176f0c1 Compare May 22, 2024 20:25
@jasondiazg
Copy link
Contributor Author

Hi @vinzscam would you mind rerunning the checks and if everything is correct, approve the MR?

@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch 2 times, most recently from c27f9a8 to 59ec5f3 Compare May 22, 2024 22:27
@jasondiazg
Copy link
Contributor Author

Hi guys, @sabre1041 @vinzscam any update on this?

sabre1041
sabre1041 reviewed May 25, 2024
View reviewed changes
Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

Functionality looks good. Quick comment related to the schema

charts/backstage/values.schema.tmpl.json Show resolved Hide resolved
@sabre1041
Copy link
Contributor

@jasondiazg can you also update the chart version?

@jasondiazg
Copy link
Contributor Author

@jasondiazg can you also update the chart version?

Yes, I did it, now my branch has the next version: 1.9.5 I hope it will be merged before a new version is released.

@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch from 0fa9a77 to 4aa876c Compare May 27, 2024 17:20
@jasondiazg jasondiazg requested a review from sabre1041 May 28, 2024 00:22
@jasondiazg
Copy link
Contributor Author

Hi @sabre1041 any update on this?

@sabre1041
Copy link
Contributor

@jasondiazg looks like it needs another bump. if you can perform the update and update the issue, ill merge it in.

Thanks for your contribution!

@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch from 4aa876c to f708fb9 Compare June 3, 2024 23:35
@jasondiazg
Copy link
Contributor Author

@jasondiazg looks like it needs another bump. if you can perform the update and update the issue, ill merge it in.

Thanks for your contribution!

Hi @sabre1041! Are you sure? I can see that the main branch has version 1.9.4 and my branch has 1.9.5. Please confirm, otherwise merge it, I am waiting for it 😄 thanks in advance!

@sabre1041
Copy link
Contributor

@jasondiazg looks like it needs another bump. if you can perform the update and update the issue, ill merge it in.
Thanks for your contribution!

Hi @sabre1041! Are you sure? I can see that the main branch has version 1.9.4 and my branch has 1.9.5. Please confirm, otherwise merge it, I am waiting for it 😄 thanks in advance!

The version of the chart 1.9.5 needs to be bumped in the README.md

@vinzscam vinzscam self-requested a review June 4, 2024 06:50
Jason Diaz G. and others added 5 commits June 4, 2024 13:04
@jasondiazg
feat(ingress): add path as property configurable
5e7928f 
Signed-off-by: Jason Diaz G. <jasondiazg@gmail.com>
Signed-off-by: Jason Diaz <jasondiazg@gmail.com>
@jasondiazg
fix(docs): Add the correct expected ingress default value in README.m…
80d8985 
…d file

Signed-off-by: Jason Diaz <jasondiazg@gmail.com>
@jasondiazg
chore(chart-version): Bump chart version to 1.9.5
8d8930b 
Signed-off-by: Jason Diaz <jasondiazg@gmail.com>
@jasondiazg
fix(schema): Change ingress path examples in chart schema
b684ae8 
Signed-off-by: Jason Diaz <jasondiazg@gmail.com>
@jasondiazg
chore(chart-version): Bump chart version to 1.9.5 in README.md
0c23c67
@jasondiazg jasondiazg force-pushed the ingress-path-configurable branch from f708fb9 to 0c23c67 Compare June 4, 2024 19:05
@jasondiazg
Copy link
Contributor Author

@jasondiazg looks like it needs another bump. if you can perform the update and update the issue, ill merge it in.
Thanks for your contribution!

Hi @sabre1041! Are you sure? I can see that the main branch has version 1.9.4 and my branch has 1.9.5. Please confirm, otherwise merge it, I am waiting for it 😄 thanks in advance!

The version of the chart 1.9.5 needs to be bumped in the README.md

Ohhhh got it sorry. Now it's done.

sabre1041
sabre1041 approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@sabre1041 sabre1041 merged commit b77ce14 into backstage:main Jun 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sabre1041 sabre1041 sabre1041 approved these changes

@vinzscam vinzscam Awaiting requested review from vinzscam

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

Successfully merging this pull request may close these issues.

Path cannot be configurable in Ingress
2 participants
@jasondiazg @sabre1041