Skip to content

Conversation

fanminshi
Copy link
Contributor

@fanminshi fanminshi commented Aug 30, 2018

Test TLS Util for the case where only the application TLS asset exists in cluster and its corresponding CA doesn't. In this situation, the CertGenerator can't proceed because it can't generate a new CA as the new CA can't be used to verify the existing application TLS asset. Therefore, CertGenerator returns an error to the caller.

cc/ @hasbro17

@fanminshi fanminshi mentioned this pull request Aug 30, 2018
10 tasks
@fanminshi fanminshi changed the title *: test the case where only the application TLS asset exist *: test the case where only the application TLS asset exists Aug 30, 2018
*: test the case when both app and CA TLS assets exist
@fanminshi
Copy link
Contributor Author

fixed CI failure. PTAL

}
if !reflect.DeepEqual(caSecret, actualCaSecret) {
t.Fatalf("expect %+v, got %+v", caSecret, actualCaSecret)
expErrMsg := "ca secret and configMap are not found"
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're matching the exact error message then its best to define this a constant in pkg/tlsutil so we can reuse it here. Instead of defining it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good!

@hasbro17
Copy link
Contributor

LGTM after nits

@fanminshi
Copy link
Contributor Author

merging when CI turns green.

@fanminshi fanminshi merged commit 391fa55 into operator-framework:master Aug 30, 2018
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.

3 participants