Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow customers to annotate CloudCredential resource on update #3575

Merged
merged 7 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .sha256sum
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ b1f1de0fe40d05de90742b17928968923b936adc294000f58974f50a297581dd swagger/redhat
c023515341196746454c0ae7af077d40d3ec13f6b88b33cb558f0a7ab17a5a24 swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/preview/2023-07-01-preview/redhatopenshift.json
440748951dd1c3b34b5ccbdcb7cd966e3b89490887a1f1d64429561fad789515 swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/stable/2023-09-04/redhatopenshift.json
74a46fdde6ceb0121fe1515c7e11e902dd921b54cffe693307fb02b3dc88f26e swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/stable/2023-11-22/redhatopenshift.json
a27184734436629e24b344c3b5c015437f144e18e7eddce7e252a1ed4cda7bca swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/preview/2024-08-12-preview/redhatopenshift.json
76e20d8da5c40013e0b5133bb454ba48a86aab9ad9563f53c8d32b7526bebc19 swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/preview/2024-08-12-preview/redhatopenshift.json
4 changes: 4 additions & 0 deletions pkg/api/admin/openshiftcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,13 @@ type IngressProfile struct {

// PlatformWorkloadIdentityProfile encapsulates all information that is specific to workload identity clusters.
type PlatformWorkloadIdentityProfile struct {
UpgradeableTo *UpgradeableTo `json:"upgradeableTo,omitempty"`
PlatformWorkloadIdentities []PlatformWorkloadIdentity `json:"platformWorkloadIdentities,omitempty"`
}

// UpgradeableTo stores a single OpenShift version a workload idetntiy cluster can be upgraded to
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
type UpgradeableTo string

// PlatformWorkloadIdentity stores information representing a single workload identity.
type PlatformWorkloadIdentity struct {
OperatorName string `json:"operatorName,omitempty"`
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/openshiftcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,9 +773,13 @@ type HiveProfile struct {
type PlatformWorkloadIdentityProfile struct {
MissingFields

UpgradeableTo *UpgradeableTo `json:"upgradeableTo,omitempty"`
PlatformWorkloadIdentities []PlatformWorkloadIdentity `json:"platformWorkloadIdentities,omitempty"`
}

// UpgradeableTo stores a single OpenShift version a workload idetntiy cluster can be upgraded to
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
type UpgradeableTo string

// PlatformWorkloadIdentity stores information representing a single workload identity.
type PlatformWorkloadIdentity struct {
MissingFields
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/v20240812preview/openshiftcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,13 @@ type IngressProfile struct {

// PlatformWorkloadIdentityProfile encapsulates all information that is specific to workload identity clusters.
type PlatformWorkloadIdentityProfile struct {
UpgradeableTo *UpgradeableTo `json:"upgradeableTo,omitempty"`
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
PlatformWorkloadIdentities []PlatformWorkloadIdentity `json:"platformWorkloadIdentities,omitempty"`
}

// UpgradeableTo stores a single OpenShift version a workload idetntiy cluster can be upgraded to
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
type UpgradeableTo string

// PlatformWorkloadIdentity stores information representing a single workload identity.
type PlatformWorkloadIdentity struct {
OperatorName string `json:"operatorName,omitempty"`
Expand Down
12 changes: 12 additions & 0 deletions pkg/api/v20240812preview/openshiftcluster_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac

if oc.Properties.PlatformWorkloadIdentityProfile != nil && oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities != nil {
out.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{}

if oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo != nil {
temp := UpgradeableTo(*oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo)
out.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo = &temp
}

out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = make([]PlatformWorkloadIdentity, len(oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities))

for i := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
Expand Down Expand Up @@ -225,6 +231,12 @@ func (c openShiftClusterConverter) ToInternal(_oc interface{}, out *api.OpenShif
}
if oc.Properties.PlatformWorkloadIdentityProfile != nil && oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities != nil {
out.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{}

if oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo != nil {
temp := api.UpgradeableTo(*oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo)
out.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo = &temp
}

out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities = make([]api.PlatformWorkloadIdentity, len(oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities))

for i := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/v20240812preview/openshiftcluster_validatestatic.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net"
"net/http"
"net/url"
"regexp"
"strings"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
Expand Down Expand Up @@ -442,6 +443,14 @@ func (sv openShiftClusterStaticValidator) validatePlatformWorkloadIdentityProfil
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.PlatformWorkloadIdentities[%d].resourceID", path, n), "Resource must be a user assigned identity.")
}
}

if pwip.UpgradeableTo != nil {
matches, err := regexp.MatchString(`^4\.[0-9]{2}\.`, string(*pwip.UpgradeableTo))
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
if !matches || err != nil {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("%s.UpgradeableTo[%v]", path, *pwip.UpgradeableTo), "UpgradeableTo must be a valid OpenShift version.")
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
}
}

return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,9 @@ func TestOpenShiftClusterStaticValidateDelta(t *testing.T) {
}

func TestOpenShiftClusterStaticValidatePlatformWorkloadIdentityProfile(t *testing.T) {
validUpgradeableToValue := UpgradeableTo("4.14.29")
invalidUpgradeableToValue := UpgradeableTo("16.107.90")

createTests := []*validateTest{
{
name: "valid empty workloadIdentityProfile",
Expand Down Expand Up @@ -1323,6 +1326,27 @@ func TestOpenShiftClusterStaticValidatePlatformWorkloadIdentityProfile(t *testin
},
wantErr: "400: InvalidParameter: properties.servicePrincipalProfile: Must provide either an identity or service principal credentials.",
},
{
name: "valid UpgradeableTo value",
modify: func(oc *OpenShiftCluster) {
oc.Identity = &Identity{}
oc.Properties.ServicePrincipalProfile = nil
oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
UpgradeableTo: &validUpgradeableToValue,
}
},
},
{
name: "invalid UpgradeableTo value",
modify: func(oc *OpenShiftCluster) {
oc.Identity = &Identity{}
oc.Properties.ServicePrincipalProfile = nil
oc.Properties.PlatformWorkloadIdentityProfile = &PlatformWorkloadIdentityProfile{
UpgradeableTo: &invalidUpgradeableToValue,
}
},
wantErr: `400: InvalidParameter: properties.platformWorkloadIdentityProfile.UpgradeableTo[16.107.90]: UpgradeableTo must be a valid OpenShift version.`,
},
}

runTests(t, testModeCreate, createTests)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/cluster/arooperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ func (m *manager) ensureCredentialsRequest(ctx context.Context) error {
return m.aroOperatorDeployer.CreateOrUpdateCredentialsRequest(ctx)
}

func (m *manager) ensureUpgradeAnnotations(ctx context.Context) error {
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
return m.aroOperatorDeployer.EnsureUpgradeAnnotations(ctx)
}

func (m *manager) renewMDSDCertificate(ctx context.Context) error {
return m.aroOperatorDeployer.RenewMDSDCertificate(ctx)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/cluster/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ func (m *manager) Update(ctx context.Context) error {
steps.Action(m.updateAROSecret),
steps.Action(m.restartAROOperatorMaster), // depends on m.updateOpenShiftSecret; the point of restarting is to pick up any changes made to the secret
steps.Condition(m.aroDeploymentReady, 5*time.Minute, true),
steps.Action(m.ensureUpgradeAnnotations),
steps.Action(m.reconcileLoadBalancerProfile),
}

Expand Down Expand Up @@ -524,7 +525,7 @@ func (m *manager) initializeKubernetesClients(ctx context.Context) error {
// initializeKubernetesClients initializes clients which are used
// once the cluster is up later on in the install process.
func (m *manager) initializeOperatorDeployer(ctx context.Context) (err error) {
m.aroOperatorDeployer, err = deploy.New(m.log, m.env, m.doc.OpenShiftCluster, m.arocli, m.client, m.extensionscli, m.kubernetescli)
m.aroOperatorDeployer, err = deploy.New(m.log, m.env, m.doc.OpenShiftCluster, m.arocli, m.client, m.extensionscli, m.kubernetescli, m.operatorcli)
return
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/operator/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"

"github.com/hashicorp/go-multierror"
operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand Down Expand Up @@ -56,6 +57,7 @@ type Operator interface {
Restart(context.Context, []string) error
IsRunningDesiredVersion(context.Context) (bool, error)
RenewMDSDCertificate(context.Context) error
EnsureUpgradeAnnotations(context.Context) error
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
}

type operator struct {
Expand All @@ -67,10 +69,11 @@ type operator struct {
client client.Client
extensionscli extensionsclient.Interface
kubernetescli kubernetes.Interface
operatorcli operatorclient.Interface
dh dynamichelper.Interface
}

func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli aroclient.Interface, client client.Client, extensionscli extensionsclient.Interface, kubernetescli kubernetes.Interface) (Operator, error) {
func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli aroclient.Interface, client client.Client, extensionscli extensionsclient.Interface, kubernetescli kubernetes.Interface, operatorcli operatorclient.Interface) (Operator, error) {
restConfig, err := restconfig.RestConfig(env, oc)
if err != nil {
return nil, err
Expand All @@ -89,6 +92,7 @@ func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli
client: client,
extensionscli: extensionscli,
kubernetescli: kubernetescli,
operatorcli: operatorcli,
dh: dh,
}, nil
}
Expand Down Expand Up @@ -432,6 +436,34 @@ func (o *operator) RenewMDSDCertificate(ctx context.Context) error {
return nil
}

func (o *operator) EnsureUpgradeAnnotations(ctx context.Context) error {
if o.oc.Properties.PlatformWorkloadIdentityProfile == nil ||
o.oc.Properties.ServicePrincipalProfile != nil {
cadenmarchese marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

upgradeableTo := string(*o.oc.Properties.PlatformWorkloadIdentityProfile.UpgradeableTo)
upgradeableAnnotation := "cloudcredential.openshift.io/upgradeable-to"

cloudcredentialobject, err := o.operatorcli.OperatorV1().CloudCredentials().Get(ctx, "cluster", metav1.GetOptions{})
if err != nil {
return err
}

if cloudcredentialobject.Annotations == nil {
cloudcredentialobject.Annotations = map[string]string{}
}

cloudcredentialobject.Annotations[upgradeableAnnotation] = upgradeableTo

_, err = o.operatorcli.OperatorV1().CloudCredentials().Update(ctx, cloudcredentialobject, metav1.UpdateOptions{})
if err != nil {
return err
}

return nil
}

func (o *operator) IsReady(ctx context.Context) (bool, error) {
ok, err := ready.CheckDeploymentIsReady(ctx, o.kubernetescli.AppsV1().Deployments(pkgoperator.Namespace), "aro-operator-master")()
o.log.Infof("deployment %q ok status is: %v, err is: %v", "aro-operator-master", ok, err)
Expand Down
121 changes: 121 additions & 0 deletions pkg/operator/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

"github.com/golang/mock/gomock"
configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -461,3 +463,122 @@ func TestCheckPodImageVersion(t *testing.T) {
})
}
}

func TestEnsureUpgradeAnnotations(t *testing.T) {
UpgradeableTo1 := api.UpgradeableTo("4.14.59")

for _, tt := range []struct {
name string
cluster api.OpenShiftClusterProperties
annotation map[string]string
wantAnnotation map[string]string
wantErr string
cloudCredentialsName string
}{
{
name: "nil PlatformWorkloadIdentityProfile, no version persisted in cluster document",
},
{
name: "non-nil ServicePrincipalProfile, no version persisted in cluster document",
cluster: api.OpenShiftClusterProperties{
ServicePrincipalProfile: &api.ServicePrincipalProfile{
ClientID: "",
ClientSecret: "",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{},
},
},
{
name: "no version persisted in cluster document, persist it",
cluster: api.OpenShiftClusterProperties{
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: &UpgradeableTo1,
},
},
annotation: nil,
wantAnnotation: map[string]string{
"cloudcredential.openshift.io/upgradeable-to": "4.14.59",
},
},
{
name: "cloud credential 'cluster' doesn't exist",
cluster: api.OpenShiftClusterProperties{
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: &UpgradeableTo1,
},
},
cloudCredentialsName: "oh_no",
annotation: nil,
wantAnnotation: nil,
wantErr: `cloudcredentials.operator.openshift.io "cluster" not found`,
},
{
name: "version persisted in cluster document, replace it",
cluster: api.OpenShiftClusterProperties{
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: &UpgradeableTo1,
},
},
annotation: map[string]string{
"cloudcredential.openshift.io/upgradeable-to": "4.14.02",
},
wantAnnotation: map[string]string{
"cloudcredential.openshift.io/upgradeable-to": "4.14.59",
},
},
{
name: "annotations exist, append the upgradeable annotation",
cluster: api.OpenShiftClusterProperties{
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: &UpgradeableTo1,
},
},
annotation: map[string]string{
"foo": "bar",
},
wantAnnotation: map[string]string{
"foo": "bar",
"cloudcredential.openshift.io/upgradeable-to": "4.14.59",
},
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
controller := gomock.NewController(t)
defer controller.Finish()

env := mock_env.NewMockInterface(controller)

oc := &api.OpenShiftCluster{
Properties: tt.cluster,
}

if tt.cloudCredentialsName == "" {
tt.cloudCredentialsName = "cluster"
}

cloudcredentialobject := &operatorv1.CloudCredential{
ObjectMeta: metav1.ObjectMeta{
Name: tt.cloudCredentialsName,
Annotations: tt.annotation,
},
}

o := operator{
oc: oc,
env: env,
operatorcli: operatorfake.NewSimpleClientset(cloudcredentialobject),
}

err := o.EnsureUpgradeAnnotations(ctx)
utilerror.AssertErrorMessage(t, err, tt.wantErr)
result, _ := o.operatorcli.OperatorV1().CloudCredentials().List(ctx, metav1.ListOptions{})
for _, v := range result.Items {
actualAnnotations := v.ObjectMeta.Annotations
if !reflect.DeepEqual(actualAnnotations, tt.wantAnnotation) {
t.Errorf("actual annotation: %v, wanted %v", tt.annotation, tt.wantAnnotation)
}
}
})
}
}
14 changes: 14 additions & 0 deletions pkg/util/mocks/operator/deploy/deploy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading