Skip to content

fix: pass 'tls-insecure' as a flag to the member-agent helm chart#121

Merged
helayoty merged 1 commit intoAzure:mainfrom
helayoty:tls-flag
Jun 28, 2022
Merged

fix: pass 'tls-insecure' as a flag to the member-agent helm chart#121
helayoty merged 1 commit intoAzure:mainfrom
helayoty:tls-flag

Conversation

@helayoty
Copy link
Contributor

@helayoty helayoty commented Jun 27, 2022 •
edited
Loading

Description of your changes

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

// @serbrech

@Arvindthiru
Copy link
Contributor

Arvindthiru commented Jun 27, 2022 •
edited
Loading

I'm not sure if this fixes #25, from what i understand the issue still exists this is just a functionality to enable/disable it right ?

@helayoty
Copy link
Contributor Author

helayoty commented Jun 27, 2022

I'm not sure if this fixes #25, from what i understand the issue still exists this is just a functionality to enable/disable it right ?
The idea is to have a way to update the insecure config. With this PR, the update will be easy by only updating the helm chart value.

Also as I mentioned in the issue, Stephane sees this as a better way to test both cases.

ryanzhang-oss
ryanzhang-oss reviewed Jun 28, 2022
View reviewed changes
cmd/memberagent/main.go
// TODO (mng): Remove TLSClientConfig/Insecure(true) once server CA can be passed in as flag.
TLSClientConfig: rest.TLSClientConfig{
Insecure: true,
Insecure: *tlsClientInsecure,
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss Jun 28, 2022

Choose a reason for hiding this comment

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

how can this work without the CABundle?

Copy link
Contributor Author

@helayoty helayoty Jun 28, 2022

Choose a reason for hiding this comment

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

It will not work for now. As I mentioned in the issue, this is just a step to configure the tls config that will help with the testing purposes as well.

Copy link
Contributor Author

@helayoty helayoty Jun 28, 2022

Choose a reason for hiding this comment

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

added 2 tasks to the issue #25

@helayoty helayoty mentioned this pull request Jun 28, 2022
Closed
2 tasks
Arvindthiru
Arvindthiru approved these changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@Arvindthiru Arvindthiru left a comment

Choose a reason for hiding this comment

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

Spoke with @helayoty LGTM

@helayoty helayoty merged commit 552fbd0 into Azure:main Jun 28, 2022
@helayoty helayoty deleted the tls-flag branch June 28, 2022 19:37
nwnt pushed a commit that referenced this pull request Jun 30, 2025
@jwtty
test: fix flaky capacity-based scheduling e2e (#121)
d437b2c
@zhiying-lin zhiying-lin mentioned this pull request Jul 2, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryanzhang-oss ryanzhang-oss ryanzhang-oss left review comments

@Arvindthiru Arvindthiru Arvindthiru approved these changes

@minhng22 minhng22 Awaiting requested review from minhng22

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@helayoty @Arvindthiru @ryanzhang-oss