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

add ServerConfig tags, export TLSClientAuth #176

Merged
merged 8 commits into from
Jan 12, 2024

Conversation

michel-laterman
Copy link
Contributor

What does this PR do?

add ServerConfig tags, export TLSClientAuth

Why is it important?

We want to enable mTLS args as part of elastic-agent's CLI. These args should be parsed and passed to fleet-server. YAML tags are needed to allow for serialization.

@michel-laterman michel-laterman marked this pull request as ready for review January 8, 2024 22:46
@michel-laterman michel-laterman requested a review from a team as a code owner January 8, 2024 22:46
@michel-laterman michel-laterman requested review from belimawr and fearful-symmetry and removed request for a team January 8, 2024 22:46
@pierrehilbert pierrehilbert added the Team:Elastic-Agent Label for the Agent team label Jan 9, 2024
@pierrehilbert pierrehilbert requested review from faec and removed request for belimawr January 9, 2024 12:53
CAs: []string{"/path/to/ca.crt"},
ClientAuth: TLSClientAuthNone,
},
clientAuth: TLSClientAuthNone, // FIXME test fails, it serializes to required
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is failing because of the omitempty tag on the ClientAuth attribute of the struct.
TLSClientAuthNone corresponds with a zero value, so it will not be serialized if it is set in the struct.

If we remove the omitempty tag, the "with ca" unit test will fail (none will serialize, and the Unpack method will not insert the required val (current default when a CA is specified).
I can try to change ClientAuth to be a pointer value instead, so there is a difference between no value, and a none (0) val

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Looks good.

Thanks for all the additional testing.

@michel-laterman michel-laterman merged commit f5fabbc into elastic:main Jan 12, 2024
6 checks passed
@michel-laterman michel-laterman deleted the tls-client-auth branch January 12, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants