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.

Sign up for GitHub

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

Jump to bottom

Add Canary TLS support #5086

Merged
merged 8 commits into from
Mar 17, 2023
Merged

Add Canary TLS support #5086

merged 8 commits into from
Mar 17, 2023

Conversation

charlese-instaclustr
Copy link
Contributor

What changed?

Added the option for Cadence Canary to connect to Cadence over TLS via the grpc port. This requires including the filepath of an appropriate CA file in the Canary config.

Why?

Cadence is moving towards primarily support communication over grpc. TLS-enabled Cadence clusters can only conform with this standard while using Canary if Canary supports TLS-encrypted connections.

How did you test it?

Verified this change by running my branch of canary against a TLS-enabled Cadence cluster. Confirmed that canary logs show a successful connection with the cluster, and that workflow_success counter metrics increase for the cadence_canary domain.

Regression tested by running my branch of canary against a non-TLS Cadence cluster, and without using the new tlsCaFile config parameter. Again, found that canary logs show a successful connection with the cluster, and that workflow_success counter metrics increase for the cadence_canary domain.

Potential risks

In the worst case, this change could affect existing connections between canary and cadence in production; however, changes to the current path have been minimised, making this unlikely.

If the new logic were to not work, only users who attempted to leverage the new TLS capability of canary would be impacted, by Canary being unable to connect.

Release notes

No configuration changes are required, so not notable.

Documentation Changes

None required.

@coveralls
Copy link

coveralls commented Mar 13, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 0186f0e1-2164-4d02-b694-037d7c080fe2

  • 0 of 21 (0.0%) changed or added relevant lines in 1 file are covered.
  • 94 unchanged lines in 17 files lost coverage.
  • Overall coverage decreased (-0.03%) to 57.082%
Changes Missing Coverage Covered Lines Changed/Added Lines %
canary/runner.go 0 21 0.0%
Files with Coverage Reduction New Missed Lines %
service/history/queue/transfer_queue_processor_base.go 1 77.62%
client/history/client.go 2 38.1%
client/history/metricClient.go 2 45.3%
common/task/weightedRoundRobinTaskScheduler.go 2 89.12%
service/history/handler.go 2 47.15%
service/history/task/transfer_active_task_executor.go 2 72.15%
service/history/task/transfer_standby_task_executor.go 2 86.4%
service/matching/matcher.go 2 90.65%
common/persistence/statsComputer.go 3 93.57%
common/persistence/serialization/parser.go 4 62.41%
Totals Coverage Status
Change from base Build 0186ed8c-939f-4cc5-9459-fac2c80653f9: -0.03%
Covered Lines: 85233
Relevant Lines: 149317
💛 - Coveralls

@charlese-instaclustr
Copy link
Contributor Author

Hi @Shaddoll @shijiesheng (just tagging for visibility as you have both been active in the thread -- thanks for that),
This seems to be approved and the tests passing, could you please give me some support in getting this change merged in? Thanks in advance!

@Shaddoll Shaddoll enabled auto-merge (squash) March 17, 2023 18:42
@Shaddoll Shaddoll merged commit a3e2774 into uber:master Mar 17, 2023
davidporter-id-au added a commit that referenced this pull request Mar 18, 2023
@davidporter-id-au
Squashed commit of the following:
a31ccba 
commit f1e2476
Author: sonpham96 <sonpham1996@gmail.com>
Date:   Sat Mar 18 05:32:01 2023 +0700

    Upgrade Golang base image to 1.18 to remediate CVEs (#5035)

    Co-authored-by: David Porter <david.porter@uber.com>

commit 1519ace
Author: charlese-instaclustr <76502507+charlese-instaclustr@users.noreply.github.com>
Date:   Fri Mar 17 22:11:27 2023 +0000

    Fix type validation in configstore DC client value updating (#5110)

    * Remove misleading type check, Add more detailed log message

    * removing debugging logging

    * Handle nil update edge case

    ---------

    Co-authored-by: allenchen2244 <102192478+allenchen2244@users.noreply.github.com>
    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit a3e2774
Author: charlese-instaclustr <76502507+charlese-instaclustr@users.noreply.github.com>
Date:   Fri Mar 17 19:02:40 2023 +0000

    Add Canary TLS support (#5086)

    * add support for TLS connections by Canary, add development config for Canary with TLS

    * update README to include new config option

    * remove testing config

    ---------

    Co-authored-by: David Porter <david.porter@uber.com>
    Co-authored-by: Shijie Sheng <shengs@uber.com>
    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit ff4eab2
Author: Shijie Sheng <shengs@uber.com>
Date:   Thu Mar 16 20:10:54 2023 -0700

    [history] more cautious in deciding domain state to make decisions on dropping queued tasks (#5164)

    What changed?

    When domain cache returned entity not found error, don't drop queued tasks to be more conservative.

    Why?

    In cases when the cache is dubious, we shouldn't drop the queued tasks.

commit 55a8d93
Author: neil-xie <104041627+neil-xie@users.noreply.github.com>
Date:   Thu Mar 16 14:18:35 2023 -0700

    Add Pinot docker files, table config and schema (#5163)

    * Initial checkin for pinot config files

commit 1304570
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Thu Mar 16 15:20:29 2023 +0200

    Set poll interval for filebased dynamic config if not set (#5160)

    * Set poll interval for filebased dynamic config if not set

    * update unit test

commit 42a14b1
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Thu Mar 16 10:49:21 2023 +0200

    Elasticsearch: reduce code duplication (#5137)

    * Elasticsearch: reduce code duplication

    * address comments

    ---------

    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit cbf0d14
Author: bowen xiao <xbowen@uber.com>
Date:   Wed Mar 15 10:19:34 2023 -0700

    fix samples documentation (#5088)

commit ba19a29
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Wed Mar 15 12:52:29 2023 +0200

    Add ShardID to valid attributes (#5161)

commit a25cba8
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Wed Mar 15 10:56:50 2023 +0200

    ES: single interface for different ES/OpenSearch versions (#5158)

    * ES: single interface for different ES/OpenSearch versions

    * make fmt

commit e3ac246
Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com>
Date:   Tue Mar 14 12:47:40 2023 -0700

    added logging with workflow/domain tags (#5159)

commit 9581488
Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com>
Date:   Mon Mar 13 16:56:45 2023 -0700

    Consistent query pershard metric (#5143)

    * added and update consistent query per shard metric

    * testing pershard metric

    * move sample logger into persistence metric client for cleaness

    * fix test

    * fix lint

    * fix test again

    * fix lint

    * sample logging with workflowid tag

    * added domain tag to logger

    * metric completed

    * addressing comments

    * fix lint

    * Revert "fix lint"

    This reverts commit 1e96944.

    * fix lint second attempt

    ---------

    Co-authored-by: Allen Chen <allenchen2244@uber.com>
davidporter-id-au added a commit that referenced this pull request Mar 30, 2023
@davidporter-id-au
Squashed commit of the following:
9f8a8d7 
commit 9d01035
Author: allenchen2244 <102192478+allenchen2244@users.noreply.github.com>
Date:   Wed Mar 29 20:50:38 2023 -0700

    large workflow hot shard detection (#5166)

    Metrics for large workflows

commit dd51c53
Author: David Porter <david.porter@uber.com>
Date:   Wed Mar 29 18:30:06 2023 -0700

    fix build (#5180)

commit 7b281c2
Author: David Porter <david.porter@uber.com>
Date:   Mon Mar 27 10:38:37 2023 -0700

    Adds a small test to catch issues with deadlocks (#5171)

    * Adds a small test to catch issues with deadlocks

commit f1e2476
Author: sonpham96 <sonpham1996@gmail.com>
Date:   Sat Mar 18 05:32:01 2023 +0700

    Upgrade Golang base image to 1.18 to remediate CVEs (#5035)

    Co-authored-by: David Porter <david.porter@uber.com>

commit 1519ace
Author: charlese-instaclustr <76502507+charlese-instaclustr@users.noreply.github.com>
Date:   Fri Mar 17 22:11:27 2023 +0000

    Fix type validation in configstore DC client value updating (#5110)

    * Remove misleading type check, Add more detailed log message

    * removing debugging logging

    * Handle nil update edge case

    ---------

    Co-authored-by: allenchen2244 <102192478+allenchen2244@users.noreply.github.com>
    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit a3e2774
Author: charlese-instaclustr <76502507+charlese-instaclustr@users.noreply.github.com>
Date:   Fri Mar 17 19:02:40 2023 +0000

    Add Canary TLS support (#5086)

    * add support for TLS connections by Canary, add development config for Canary with TLS

    * update README to include new config option

    * remove testing config

    ---------

    Co-authored-by: David Porter <david.porter@uber.com>
    Co-authored-by: Shijie Sheng <shengs@uber.com>
    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit ff4eab2
Author: Shijie Sheng <shengs@uber.com>
Date:   Thu Mar 16 20:10:54 2023 -0700

    [history] more cautious in deciding domain state to make decisions on dropping queued tasks (#5164)

    What changed?

    When domain cache returned entity not found error, don't drop queued tasks to be more conservative.

    Why?

    In cases when the cache is dubious, we shouldn't drop the queued tasks.

commit 55a8d93
Author: neil-xie <104041627+neil-xie@users.noreply.github.com>
Date:   Thu Mar 16 14:18:35 2023 -0700

    Add Pinot docker files, table config and schema (#5163)

    * Initial checkin for pinot config files

commit 1304570
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Thu Mar 16 15:20:29 2023 +0200

    Set poll interval for filebased dynamic config if not set (#5160)

    * Set poll interval for filebased dynamic config if not set

    * update unit test

commit 42a14b1
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Thu Mar 16 10:49:21 2023 +0200

    Elasticsearch: reduce code duplication (#5137)

    * Elasticsearch: reduce code duplication

    * address comments

    ---------

    Co-authored-by: Zijian <Shaddoll@users.noreply.github.com>

commit cbf0d14
Author: bowen xiao <xbowen@uber.com>
Date:   Wed Mar 15 10:19:34 2023 -0700

    fix samples documentation (#5088)

commit ba19a29
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Wed Mar 15 12:52:29 2023 +0200

    Add ShardID to valid attributes (#5161)

commit a25cba8
Author: Mantas Šidlauskas <mantass@netapp.com>
Date:   Wed Mar 15 10:56:50 2023 +0200

    ES: single interface for different ES/OpenSearch versions (#5158)

    * ES: single interface for different ES/OpenSearch versions

    * make fmt

commit e3ac246
Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com>
Date:   Tue Mar 14 12:47:40 2023 -0700

    added logging with workflow/domain tags (#5159)

commit 9581488
Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com>
Date:   Mon Mar 13 16:56:45 2023 -0700

    Consistent query pershard metric (#5143)

    * added and update consistent query per shard metric

    * testing pershard metric

    * move sample logger into persistence metric client for cleaness

    * fix test

    * fix lint

    * fix test again

    * fix lint

    * sample logging with workflowid tag

    * added domain tag to logger

    * metric completed

    * addressing comments

    * fix lint

    * Revert "fix lint"

    This reverts commit 1e96944.

    * fix lint second attempt

    ---------

    Co-authored-by: Allen Chen <allenchen2244@uber.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shaddoll Shaddoll Shaddoll approved these changes

@mantas-sidlauskas mantas-sidlauskas mantas-sidlauskas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@charlese-instaclustr @coveralls @mantas-sidlauskas @Shaddoll @davidporter-id-au @shijiesheng