Skip to content

feat: Add TLS support for Go Feature Server#6229

Merged
franciscojavierarceo merged 10 commits intofeast-dev:masterfrom
shuchu:feat/issue-6095
Apr 14, 2026
Merged

feat: Add TLS support for Go Feature Server#6229
franciscojavierarceo merged 10 commits intofeast-dev:masterfrom
shuchu:feat/issue-6095

Conversation

@shuchu
Copy link
Copy Markdown
Collaborator

@shuchu shuchu commented Apr 5, 2026 •
edited
Loading

What this PR does / why we need it:

Add TLS support for users who have enterprise security requirement

Which issue(s) this PR fixes:

Fix #6095

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Misc

Open with Devin

@shuchu shuchu requested a review from a team as a code owner April 5, 2026 23:59
devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

@shuchu shuchu marked this pull request as draft April 6, 2026 00:36
@shuchu shuchu changed the title Add TLS support for Go Feature Server feat: Add TLS support for Go Feature Server Apr 10, 2026
@shuchu shuchu marked this pull request as ready for review April 10, 2026 03:46
devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

ntkathole
ntkathole reviewed Apr 11, 2026
View reviewed changes
Comment thread go/internal/feast/server/http_server.go Outdated
@ntkathole
Copy link
Copy Markdown
Member

ntkathole commented Apr 11, 2026

@shuchu optional nitpick - I see duplication of code in http vs https in start and serve methods

@shuchu
Copy link
Copy Markdown
Collaborator Author

shuchu commented Apr 12, 2026

@shuchu optional nitpick - I see duplication of code in http vs https in start and serve methods

yes, you are right. let me put those two functions into one.

devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

@shuchu shuchu requested a review from ntkathole April 14, 2026 01:15
shuchu added 10 commits April 13, 2026 22:44
@shuchu
feat: Add HTTPs server support.
7dce12f 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
feat: Add serverTSL() method.
064f612 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Lint the Go code.
5d64fcb 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
feat: Add TLS configuration.
ed19573 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Add unit tests for HTTPS server.
2ca0bba 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Format code. Fixed bad comments.
531042a 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Fix unit test error.
d87d895 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: use single StartHttpServer to cover HTTP and HTTPs cases.
c6f5d69 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Change back the pyproject.toml file.
f3d93c5 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
@shuchu
fix: Fix name convertion of https_enable.
03d7156 
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
devin-ai-integration[bot]
devin-ai-integration bot reviewed Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 13 additional findings in Devin Review.

Open in Devin Review

Comment thread go/main_test.go
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 14, 2026

Choose a reason for hiding this comment

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

🟡 MockServerStarter missing StartHttpsServer method required by ServerStarter interface

The ServerStarter interface at go/main.go:51 now includes StartHttpsServer, but MockServerStarter in go/main_test.go:29 does not implement this method. The comment on line 28 explicitly states it is a mock of ServerStarter interface for testing, yet it no longer satisfies that interface. Any test that attempts to use MockServerStarter as a ServerStarter will fail to compile.

(Refers to lines 29-41)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@franciscojavierarceo franciscojavierarceo merged commit 28a58d0 into feast-dev:master Apr 14, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments

@franciscojavierarceo franciscojavierarceo franciscojavierarceo approved these changes

@HaoXuAI HaoXuAI Awaiting requested review from HaoXuAI

@ntkathole ntkathole Awaiting requested review from ntkathole

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add TLS configuration support to Go feature server

3 participants

@shuchu @ntkathole @franciscojavierarceo