feat: introduce v1beta1 version #526
Conversation
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
odubajDT
force-pushed
the
feat/352/beta
branch
from
1bd9ac9
to
dcbcdf1
Compare
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
odubajDT
changed the title
Codecov Report
@@ Coverage Diff @@
## main #526 +/- ##
=======================================
Coverage 82.78% 82.78%
=======================================
Files 22 22
Lines 1423 1423
=======================================
Hits 1178 1178
Misses 207 207
Partials 38 38
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
There is a problem with lint CI check, which is present on the main branch. This will be handled in a separate PR |
| Image string `json:"image"` | ||
|
|
||
| // Tag to be appended to the sidecar image, defaults to 'main' | ||
| // +optional | ||
| Tag string `json:"tag"` |
There was a problem hiding this comment.
This is related to issue #517
This can be considered as a security concern and hence we can discuss on removing these
cc - @toddbaert @thisthat appreciate your opinion
There was a problem hiding this comment.
adapted the code to remove tag and image from FlagSourceConfiguration CRD
There was a problem hiding this comment.
Ya I'm fine with removing them. I have only heard agreements on this.
I think we should come up with a short list of changes that we can include in the extended commit message.
There was a problem hiding this comment.
With further implementation I came across some issues regarding this. Read comment here #517 (comment)
I would suggest that covering this should be part of a separate PR, as it will bump the complexity of the PR, which will be adapting the code to use v1beta1 instead of v1alpha1. Any objections on this ?
There was a problem hiding this comment.
|
|
||
| const ( | ||
| SyncProviderKubernetes SyncProviderType = "kubernetes" | ||
| SyncProviderFilepath SyncProviderType = "filepath" |
There was a problem hiding this comment.
Please consider the diff highlighted in this related issue - #433
There was a problem hiding this comment.
Do we already have a unified solution? I see that the ticket does not clearly state which option to use
There was a problem hiding this comment.
@beeme1mr @Kavindu-Dodan @thisthat which is better here? Any opinions? I suppose since we're already doing breaking changes here, it makes sense to concentrate them all here... so perhaps we should conform here in OFO to the flagd style: file.
odubajDT
force-pushed
the
feat/352/beta
branch
from
fbcf24d
to
583669e
Compare
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
|
Closing in favor of #535 |
This PR
Related Issues
Part-of #352
Part-of #433
Follow-up
Notes
There is a problem with lint CI check, which is present on the main branch. This will be handled in a separate PR