chore: refactor configuration handling for startup by Kavindu-Dodan · Pull Request #551 · open-feature/flagd

Kavindu-Dodan · 2023-03-22T21:51:49Z

This PR

No logic changes - Pure refactoring[1] to untangle startup argument-based configurations from runtime.

Consider the simple sequence diagram below,

sequenceDiagram
    participant start
    participant from_config
    participant runtime
    start-->>from_config: ParseSyncProviderURIs()
    start-->>from_config: ParseSources()
    start-->>from_config: FromConfig()
    Note left of from_config: This parse & build the runtime.
    from_config-->>start: Runtime
    Note left of from_config: Runtime structure is self-contained & <br/> does not rely on the startup configuration
    start-->>runtime: rt.Start()
    loop Run
        runtime->>runtime: flagd runtime
    end

The core idea was to isolate startup configuration & get rid of the unwanted Config [2] structure parsing to the Runtime layer.

This change

Helps to isolate startup arguments & runtime (low coupling)
Improves testability by removing unused/unwanted pointer receivers
Improves readability as we have a clear logic flow

[1] Note - Had to change this faulty log line :

flagd/core/pkg/runtime/from_config.go

Lines 149 to 151 in c98db4f

    
           default: 
        
           	return nil, fmt.Errorf("invalid sync provider: %s, must be one of with '%s', '%s', '%s' or '%s'", 
        
           		syncProvider.Provider, syncProviderFile, syncProviderKubernetes, syncProviderHTTP, syncProviderKubernetes)

[2] - https://github.com/open-feature/flagd/blob/main/core/pkg/runtime/runtime.go#L30

codecov · 2023-03-22T21:55:30Z

Codecov Report

Merging #551 (00f2da8) into main (c7b29ed) will increase coverage by 2.62%.
The diff coverage is 41.17%.

@@            Coverage Diff             @@
##             main     #551      +/-   ##
==========================================
+ Coverage   66.77%   69.40%   +2.62%     
==========================================
  Files          19       19              
  Lines        2125     2121       -4     
==========================================
+ Hits         1419     1472      +53     
+ Misses        646      589      -57     
  Partials       60       60

Impacted Files	Coverage Δ
core/pkg/runtime/runtime.go	`0.00% <0.00%> (ø)`
core/pkg/runtime/from_config.go	`58.92% <43.07%> (+31.04%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

odubajDT

LGTM! :)

toddbaert · 2023-03-23T14:56:09Z

-	syncProvidersParsed := []sync.SourceConfig{}
-	if err := json.Unmarshal([]byte(syncProviders), &syncProvidersParsed); err != nil {
+// ParseSources parse a json formatted SourceConfig array string and performs validations on the content
+func ParseSources(sourcesFlag string) ([]SourceConfig, error) {


Much better names, thanks.

toddbaert · 2023-03-23T14:57:46Z

+			return nil, fmt.Errorf("invalid sync provider: %s, must be one of with '%s', '%s', '%s' or '%s'",
+				syncProvider.Provider, syncProviderFile, syncProviderKubernetes, syncProviderHTTP, syncProviderKubernetes)


I guess this is the incorrect log you were mentioning.

Correct. Here the switch work on syncProvider.Provider & not the URI :)

🤖 I have created a release *beep* *boop* --- <details><summary>flagd: 0.5.0</summary> ## [0.5.0](flagd/v0.4.5...flagd/v0.5.0) (2023-03-30) ### ⚠ BREAKING CHANGES * unify sources configuration handling ([#560](#560)) ### 🐛 Bug Fixes * benchmark pipeline ([#538](#538)) ([62cc0fc](62cc0fc)) * **deps:** update module github.com/open-feature/flagd/core to v0.4.5 ([#552](#552)) ([41799f6](41799f6)) ### 🧹 Chore * refactor configuration handling for startup ([#551](#551)) ([8dfbde5](8dfbde5)) </details> <details><summary>flagd-proxy: 0.2.0</summary> ## [0.2.0](flagd-proxy-v0.1.2...flagd-proxy/v0.2.0) (2023-03-30) ### ⚠ BREAKING CHANGES * rename `kube-flagd-proxy` to `flagd-proxy` ([#576](#576)) ### ✨ New Features * rename `kube-flagd-proxy` to `flagd-proxy` ([#576](#576)) ([223de99](223de99)) </details> <details><summary>core: 0.5.0</summary> ## [0.5.0](core/v0.4.5...core/v0.5.0) (2023-03-30) ### ⚠ BREAKING CHANGES * rename `kube-flagd-proxy` to `flagd-proxy` ([#576](#576)) * unify sources configuration handling ([#560](#560)) ### 🧹 Chore * move credential builder for grpc sync into seperate component ([#536](#536)) ([7314fee](7314fee)) * refactor configuration handling for startup ([#551](#551)) ([8dfbde5](8dfbde5)) * refactor middleware setup in server ([#554](#554)) ([01016c7](01016c7)) * refactor service configuration objects ([#545](#545)) ([c7b29ed](c7b29ed)), closes [#524](#524) * unify sources configuration handling ([#560](#560)) ([7f4888a](7f4888a)) ### 🐛 Bug Fixes * **deps:** update module google.golang.org/grpc to v1.54.0 ([#548](#548)) ([99ba5ec](99ba5ec)) * **deps:** update module sigs.k8s.io/controller-runtime to v0.14.6 ([#572](#572)) ([bed9458](bed9458)) * fixing silent lint failures ([#550](#550)) ([30c8022](30c8022)) * nil pointer fix + export constructors ([#555](#555)) ([78adb81](78adb81)) ### ✨ New Features * expose Impression metric ([#556](#556)) ([77e0a33](77e0a33)) * Introduce kube-proxy-metrics ([#558](#558)) ([ad0baeb](ad0baeb)) * rename `kube-flagd-proxy` to `flagd-proxy` ([#576](#576)) ([223de99](223de99)) * refactor core module into multiple packages ([#530](#530)) ([9d68d0b](9d68d0b)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: Todd Baert <toddbaert@gmail.com> Co-authored-by: Todd Baert <toddbaert@gmail.com>

Kavindu-Dodan requested review from AlexsJones, james-milligan and toddbaert as code owners March 22, 2023 21:51

Kavindu-Dodan requested review from skyerus and thisthat March 22, 2023 21:52

Kavindu-Dodan force-pushed the chore/refactor-config-handling branch 2 times, most recently from 3ad9c86 to 0f171eb Compare March 23, 2023 13:28

Kavindu-Dodan added 3 commits March 23, 2023 07:10

refactor

4cf99b1

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

add tests and comments

24e815c

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

lint fixes

70e7bae

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

Kavindu-Dodan force-pushed the chore/refactor-config-handling branch from 0f171eb to 9975c90 Compare March 23, 2023 14:20

fix conflicts

00f2da8

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

Kavindu-Dodan force-pushed the chore/refactor-config-handling branch from 9975c90 to 00f2da8 Compare March 23, 2023 14:29

odubajDT approved these changes Mar 23, 2023

View reviewed changes

toddbaert reviewed Mar 23, 2023

View reviewed changes

toddbaert self-requested a review March 23, 2023 14:57

toddbaert approved these changes Mar 23, 2023

View reviewed changes

toddbaert merged commit 8dfbde5 into open-feature:main Mar 23, 2023

github-actions Bot mentioned this pull request Mar 23, 2023

chore: release main #547

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: refactor configuration handling for startup#551

chore: refactor configuration handling for startup#551
toddbaert merged 4 commits intoopen-feature:mainfrom
Kavindu-Dodan:chore/refactor-config-handling

Kavindu-Dodan commented Mar 22, 2023 •

edited

Loading

Uh oh!

codecov Bot commented Mar 22, 2023 •

edited

Loading

Uh oh!

odubajDT left a comment

Uh oh!

toddbaert Mar 23, 2023

Uh oh!

toddbaert Mar 23, 2023

Uh oh!

Kavindu-Dodan Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	default:
	return nil, fmt.Errorf("invalid sync provider: %s, must be one of with '%s', '%s', '%s' or '%s'",
	syncProvider.Provider, syncProviderFile, syncProviderKubernetes, syncProviderHTTP, syncProviderKubernetes)

Conversation

Kavindu-Dodan commented Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR

Uh oh!

codecov Bot commented Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

odubajDT left a comment

Choose a reason for hiding this comment

Uh oh!

toddbaert Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

toddbaert Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

Kavindu-Dodan Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kavindu-Dodan commented Mar 22, 2023 •

edited

Loading

codecov Bot commented Mar 22, 2023 •

edited

Loading