-
Notifications
You must be signed in to change notification settings - Fork 22
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
Copy over annotations to ingress #7
Conversation
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Please could you reset/squash the commits down to remove the merge commit? That function should be easy to test. I wrote some tests but they don't appear to be in the repo. Try this: https://blog.alexellis.io/golang-writing-unit-tests/ |
Signed-off-by: Marcus Noble <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks in great shape, just a couple of nits for maintenance now, a separate commit will be fine for these changes.
Thanks for adding the unit tests, you didn't resolve, or reply to the comment. That's something you can do to help me.
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Signed-off-by: Marcus Noble <[email protected]>
@alexellis Are there any more changes requested for this PR? |
} | ||
annotations["kubernetes.io/ingress.class"] = class | ||
annotations["com.openfaas.spec"] = string(specJSON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarcusNoble should you take specJSON
after setting the ingress.class?
Thank you for picking this up again :) I have left a comment |
Thank you @MarcusNoble 💪 0.4.0 is now out with the changes. |
Signed-off-by: Marcus Noble [email protected]
Description
Copy the annotations from FunctionIngress to the Ingress object that gets created so that ingress-specific annotations can be used (for example marking the ingress as being internal)
Motivation and Context
I am currently looking to expose a function within out work network, using an internal ALB but an annotation on the ingress resource is needed for this.
How Has This Been Tested?
Test yaml:
Types of changes
Impact to existing users
There shouldn't be any impact.
Checklist:
git commit -s