Skip to content

fix: nil pointer fix + export constructors#555

Merged
beeme1mr merged 4 commits intoopen-feature:mainfrom
james-milligan:main
Mar 24, 2023
Merged

fix: nil pointer fix + export constructors#555
beeme1mr merged 4 commits intoopen-feature:mainfrom
james-milligan:main

Conversation

@james-milligan
Copy link
Copy Markdown
Contributor

@james-milligan james-milligan commented Mar 23, 2023

This PR

  • fixes nil pointer deference bug with grpc sync
  • exports constructors, this code was duplicated in the sync store

Related Issues

Fixes #1234523

Notes

go run flagd/main.go start --debug --sources '[{"uri":"grpc://localhost:8013","provider":"grpc","selector":"file:./config/samples/example_flags.json"}]'

resulted in

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x1047af4e8]

goroutine 1 [running]:
github.com/open-feature/flagd/core/pkg/sync/grpc.(*Sync).Init(0x140002671f0, {0x10582c6b0, 0x14000687450})

credential builder was not being set in the constructor

Follow-up Tasks

How to test

@james-milligan
nil pointer fix + export constructors
51b1ac2 
Signed-off-by: James Milligan <james@omnant.co.uk>
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2023 •
edited
Loading

Codecov Report

Merging #555 (889be49) into main (b0818aa) will increase coverage by 0.37%.
The diff coverage is 45.45%.

@@            Coverage Diff             @@
##             main     #555      +/-   ##
==========================================
+ Coverage   69.40%   69.77%   +0.37%     
==========================================
  Files          19       19              
  Lines        2121     2111      -10     
==========================================
+ Hits         1472     1473       +1     
+ Misses        589      578      -11     
  Partials       60       60
Impacted Files Coverage Δ
core/pkg/sync-store/sync_store.go 88.70% <0.00%> (+5.18%) ⬆️
core/pkg/runtime/from_config.go 59.17% <83.33%> (+0.24%) ⬆️

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

@james-milligan
merge
e20b47f 
Signed-off-by: James Milligan <james@omnant.co.uk>
@james-milligan
fix conflicts
3faddd7 
Signed-off-by: James Milligan <james@omnant.co.uk>
@james-milligan james-milligan requested a review from a team March 24, 2023 13:18
@beeme1mr beeme1mr self-requested a review March 24, 2023 13:31
@beeme1mr beeme1mr merged commit 78adb81 into open-feature:main Mar 24, 2023
@github-actions github-actions Bot mentioned this pull request Mar 24, 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

@beeme1mr beeme1mr beeme1mr approved these changes

@AlexsJones AlexsJones Awaiting requested review from AlexsJones

@toddbaert toddbaert Awaiting requested review from toddbaert

+2 more reviewers

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

@odubajDT odubajDT odubajDT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@james-milligan @beeme1mr @Kavindu-Dodan @odubajDT