Validation around licenseKey field for extended kubernetes version support #9218
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
eks-distro-bot
added
the
size/L
panktishah26
force-pushed
the
standardsupport-validation
branch
from
a47baf1 to
987ef11
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9218 +/- ##
==========================================
- Coverage 72.38% 72.38% -0.01%
==========================================
Files 587 587
Lines 45836 45910 +74
==========================================
+ Hits 33178 33230 +52
- Misses 10910 10932 +22
Partials 1748 1748 ☔ View full report in Codecov by Sentry. |
panktishah26
force-pushed
the
standardsupport-validation
branch
4 times, most recently
from
d4d8b0c to
c26ec97
Compare
pkg/validations/extendedversion.go
Outdated
| return dateToCompare.Before(today) | ||
| } | ||
|
|
||
| func parseDate(format string, date string) (time.Time, error) { |
There was a problem hiding this comment.
is this function really needed? it takes the exact same inputs and returns the exact same outputs as time.Parse?
pkg/validations/extendedversion.go
Outdated
| @@ -31,3 +51,60 @@ func validateBundleSignature(bundle *v1alpha1.Bundles) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func isExtendedSupport(clusterSpec *anywherev1.Cluster, bundle *v1alpha1.Bundles) (bool, error) { | |||
There was a problem hiding this comment.
this function only uses clusterSpec.Spec.KubernetesVersion from clusterSpec I recommend not taking in the whole anywherev1.Cluster object just for the kubernetes version field. I recommend just taking in what this function needs, so just the data type of clusterSpec.Spec.KubernetesVersion.
pkg/validations/extendedversion.go
Outdated
| return isPastDateThanToday(endOfStandardSupport), nil | ||
| } | ||
|
|
||
| func getLicense(clusterSpec *anywherev1.Cluster) (*jwt.Token, error) { |
There was a problem hiding this comment.
this function only uses clusterSpec.Spec.LicenseToken from clusterSpec I recommend not taking in the whole anywherev1.Cluster object just for the kubernetes version field. I recommend just taking in what this function needs, so just the data type of clusterSpec.Spec.LicenseToken.
pkg/validations/extendedversion.go
Outdated
| "github.com/aws/eks-anywhere/pkg/constants" | ||
| "github.com/aws/eks-anywhere/pkg/signature" | ||
| "github.com/aws/eks-anywhere/release/api/v1alpha1" | ||
| ) | ||
|
|
||
| // ValidateExtendedK8sVersionSupport validates all the validations needed for the support of extended kubernetes support. | ||
| func ValidateExtendedK8sVersionSupport(_ context.Context, _ *anywherev1.Cluster, bundle *v1alpha1.Bundles, _ kubernetes.Client) error { | ||
| func ValidateExtendedK8sVersionSupport(_ context.Context, clusterSpec *anywherev1.Cluster, bundle *v1alpha1.Bundles, _ kubernetes.Client) error { |
There was a problem hiding this comment.
looks like the clusterSpec is needed but only for 2 fields within it. I don't recommend taking in the whole cluster spec as a dependency.
There was a problem hiding this comment.
As discussed offline, I am not passing clusterSpec as a pointer.
panktishah26
force-pushed
the
standardsupport-validation
branch
from
c26ec97 to
78910a3
Compare
Issue #, if available:
#6793
Description of changes:
This PR adds below validations for eks-a controller,
Testing (if applicable):
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.