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

Validate config properties #683

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Validate config properties #683

wants to merge 1 commit into from

Conversation

yunkunrao
Copy link

This is related with #589

Signed-off-by: rao yunkun [email protected]

Copy link
Collaborator

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

Thanks @yunkunrao. I appreciate that you added tests to accompany this change as well. LGTM!

@thomastaylor312
Copy link
Member

thomastaylor312 commented Oct 4, 2021

@yunkunrao Looks like your tests are expecting a path that isn't there on the runner. You might need to generate a kubeconfig for the test to use

@yunkunrao
Copy link
Author

yunkunrao commented Oct 6, 2021

I have refactored this PR and add a public method is_valid() which is dedicated for config validatiion. If have any suggestions, please feel free to let me know. Thanks.

@bacongobbler
Copy link
Collaborator

bacongobbler commented Oct 6, 2021

Thanks @yunkunrao. It looks like this is failing the DCO sign-off - one of the reverted commits is missing the "Signed-off-by" header required by the DCO. See https://github.com/krustlet/krustlet/pull/683/checks?check_run_id=3816534329 for more details

I don't see any reason why the windows e2e tests failed. I'll try restarting those and see what happens. Try rebasing your commits onto main?

Signed-off-by: rao yunkun <[email protected]>
@yunkunrao
Copy link
Author

Thanks @yunkunrao. It looks like this is failing the DCO sign-off - one of the reverted commits is missing the "Signed-off-by" header required by the DCO. See https://github.com/krustlet/krustlet/pull/683/checks?check_run_id=3816534329 for more details

I don't see any reason why the windows e2e tests failed. I'll try restarting those and see what happens. Try rebasing your commits onto main?

Got it. I have squashed commits to a single one.

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.

Check that parameters for certificate and private key are not set to the same file
3 participants