Skip to content

chore!: unify sources configuration handling#560

Merged
Kavindu-Dodan merged 13 commits intoopen-feature:mainfrom
Kavindu-Dodan:chore/improve-sources-config-handling
Mar 28, 2023
Merged

chore!: unify sources configuration handling#560
Kavindu-Dodan merged 13 commits intoopen-feature:mainfrom
Kavindu-Dodan:chore/improve-sources-config-handling

Conversation

@Kavindu-Dodan
Copy link
Copy Markdown
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Mar 24, 2023 •
edited
Loading

This PR

This PR is best reviewed by going through the new documentation [1]

Restructured flagd startup configuration handling with respect to sync providers. Documentation is updated [1] & handling of prefixes is cleared (ex:- removal of grpc(s):// prefix for --source based sync provider startup argument)

Breaking change

Contains a breaking change with respect to how sync provider URIs are handled

  • what -source configuration URI parsing
  • why - align source configuration handling among different sources & avoid mix uage of prefixes to aling with documented behavior
  • how - source configuration no longer validate or parse URI prefixes. This means, existing source configuration strings/files may need an upgrade if they used prefixes in URIs. The upgrade is to simply remove any prefixes from source configuration URIs

For example,

old

sources:
- uri: grpcs://my-flag-source:8080 # mixed usage of schema shared with URI prefixes ❌ 
  provider: grpc
  certPath: /certs/ca.cert

new

sources:
- uri: my-flag-source:8080
  provider: grpc
  tls: true. # new way to convey secure connection requirement ✅ 
  certPath: /certs/ca.cert

Is a pre-requisite of open-feature/open-feature-operator#389

[1] - https://github.com/open-feature/flagd/blob/2a4783ef61eb3756e7a3f99aae596392906cd765/docs/configuration/configuration.md

@Kavindu-Dodan Kavindu-Dodan requested a review from a team March 24, 2023 16:26
@Kavindu-Dodan Kavindu-Dodan changed the title chore! : improve sources config handling chore!: improve sources config handling Mar 24, 2023
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch 2 times, most recently from 1087d0f to 4595aed Compare March 24, 2023 16:33
@Kavindu-Dodan Kavindu-Dodan changed the title chore!: improve sources config handling chore!: unify sources configuration handling Mar 24, 2023
james-milligan
james-milligan reviewed Mar 24, 2023
View reviewed changes
Comment thread core/pkg/runtime/from_config.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2023 •
edited
Loading

Codecov Report

Merging #560 (a18ce77) into main (8c00040) will decrease coverage by 0.23%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #560      +/-   ##
==========================================
- Coverage   71.04%   70.81%   -0.23%     
==========================================
  Files          21       21              
  Lines        2179     2162      -17     
==========================================
- Hits         1548     1531      -17     
  Misses        574      574              
  Partials       57       57
Impacted Files Coverage Δ
core/pkg/runtime/from_config.go 59.64% <100.00%> (+0.47%) ⬆️
core/pkg/sync/grpc/credentials/builder.go 100.00% <100.00%> (ø)
core/pkg/sync/grpc/grpc_sync.go 89.51% <100.00%> (-1.46%) ⬇️

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

james-milligan
james-milligan reviewed Mar 24, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch 3 times, most recently from 3365b9d to 643392a Compare March 24, 2023 17:41
thisthat
thisthat reviewed Mar 25, 2023
View reviewed changes
Comment thread core/pkg/runtime/from_config_test.go
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch 3 times, most recently from 0811411 to 13a7e71 Compare March 27, 2023 14:50
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md Outdated
toddbaert
toddbaert reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch from 9248b29 to 2a4783e Compare March 27, 2023 21:46
beeme1mr
beeme1mr reviewed Mar 27, 2023
View reviewed changes
Comment thread docs/configuration/configuration.md
@Kavindu-Dodan Kavindu-Dodan mentioned this pull request Mar 27, 2023
Closed
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch from 2a4783e to a21128f Compare March 28, 2023 14:13
@toddbaert toddbaert self-requested a review March 28, 2023 15:08
toddbaert
toddbaert approved these changes Mar 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

LGTM!

beeme1mr
beeme1mr approved these changes Mar 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Nice change, thanks!

@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch from a21128f to 3749331 Compare March 28, 2023 16:27
Kavindu-Dodan and others added 13 commits March 28, 2023 09:52
@Kavindu-Dodan
source config handling
f87c9e7 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan
improve test readability
a2145c7 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan
review change
72f9d23 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan
enhance documentation
845fbab 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan
review change - add more test coverage
0d35dec 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan
fix linting for md
2e33d14 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
57b2317 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
d6405ed 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
924387b 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
b0fa374 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
c4a0f8a 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan @toddbaert
Update docs/configuration/configuration.md
867e613 
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
@Kavindu-Dodan
review changes
a18ce77 
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan Kavindu-Dodan force-pushed the chore/improve-sources-config-handling branch from 3749331 to a18ce77 Compare March 28, 2023 16:52
@Kavindu-Dodan Kavindu-Dodan merged commit 7f4888a into open-feature:main Mar 28, 2023
@Kavindu-Dodan
Copy link
Copy Markdown
Contributor Author

Kavindu-Dodan commented Mar 28, 2023

merged with approvals and as all comments were resolved

@github-actions github-actions Bot mentioned this pull request Mar 28, 2023
Merged
toddbaert added a commit that referenced this pull request Mar 30, 2023
@github-actions @toddbaert
chore: release main (#547)
529e3e9 
🤖 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>
@github-actions github-actions Bot mentioned this pull request Dec 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thisthat thisthat thisthat left review comments

@james-milligan james-milligan james-milligan left review comments

@beeme1mr beeme1mr beeme1mr approved these changes

@toddbaert toddbaert toddbaert 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.

5 participants

@Kavindu-Dodan @beeme1mr @thisthat @toddbaert @james-milligan