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.

Sign up for GitHub

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

Jump to bottom

Enable contextual login in OCI HelmRepository #873

Merged
merged 1 commit into from
Aug 26, 2022
Merged

Enable contextual login in OCI HelmRepository #873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion api/v1beta2/helmrepository_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ type HelmRepositorySpec struct {
// +required
Interval metav1.Duration `json:"interval"`

// Timeout of the index fetch operation, defaults to 60s.
// Timeout is used for the index fetch operation for an HTTPS helm repository,
// and for remote OCI Repository operations like pulling for an OCI helm repository.
// Its default value is 60s.
// +kubebuilder:default:="60s"
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`
Expand All @@ -89,6 +91,14 @@ type HelmRepositorySpec struct {
// +kubebuilder:validation:Enum=default;oci
// +optional
Type string `json:"type,omitempty"`

// Provider used for authentication, can be 'aws', 'azure', 'gcp' or 'generic'.
// This field is optional, and only taken into account if the .spec.type field is set to 'oci'.
// When not specified, defaults to 'generic'.
// +kubebuilder:validation:Enum=generic;aws;azure;gcp
// +kubebuilder:default:=generic
// +optional
Provider string `json:"provider,omitempty"`
}

// HelmRepositoryStatus records the observed state of the HelmRepository.
Expand Down
16 changes: 15 additions & 1 deletion config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,18 @@ spec:
be done with caution, as it can potentially result in credentials
getting stolen in a MITM-attack.
type: boolean
provider:
default: generic
description: Provider used for authentication, can be 'aws', 'azure',
'gcp' or 'generic'. This field is optional, and only taken into
account if the .spec.type field is set to 'oci'. When not specified,
defaults to 'generic'.
enum:
- generic
- aws
- azure
- gcp
type: string
secretRef:
description: SecretRef specifies the Secret containing authentication
credentials for the HelmRepository. For HTTP/S basic auth the secret
Expand All @@ -328,7 +340,9 @@ spec:
type: boolean
timeout:
default: 60s
description: Timeout of the index fetch operation, defaults to 60s.
description: Timeout is used for the index fetch operation for an
HTTPS helm repository, and for remote OCI Repository operations
like pulling for an OCI helm repository. Its default value is 60s.
type: string
type:
description: Type of the HelmRepository. When this field is set to "oci",
Expand Down
34 changes: 34 additions & 0 deletions controllers/helmchart_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/oci"
"github.com/fluxcd/pkg/runtime/conditions"
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/events"
Expand Down Expand Up @@ -463,6 +464,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
tlsConfig *tls.Config
loginOpts []helmreg.LoginOption
)
// Used to login with the repository declared provider
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
souleb marked this conversation as resolved.
Show resolved Hide resolved
defer cancel()

normalizedURL := repository.NormalizeURL(repo.Spec.URL)
// Construct the Getter options from the HelmRepository data
Expand Down Expand Up @@ -521,6 +525,21 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
}

if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := &serror.Event{
Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
if auth != nil {
loginOpts = append([]helmreg.LoginOption{}, auth)
}
}

// Initialize the chart repository
var chartRepo repository.Downloader
switch repo.Spec.Type {
Expand Down Expand Up @@ -947,6 +966,11 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
},
}
}

// Used to login with the repository declared provider
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
defer cancel()

clientOpts := []helmgetter.Option{
helmgetter.WithURL(normalizedURL),
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
Expand Down Expand Up @@ -976,6 +1000,16 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
}

if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
return nil, fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr)
}
if auth != nil {
loginOpts = append([]helmreg.LoginOption{}, auth)
}
}

var chartRepo repository.Downloader
if helmreg.IsOCI(normalizedURL) {
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
Expand Down
7 changes: 4 additions & 3 deletions controllers/helmchart_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,9 +1085,10 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
GenerateName: "helmrepository-",
},
Spec: sourcev1.HelmRepositorySpec{
URL: fmt.Sprintf("oci://%s/testrepo", testRegistryServer.registryHost),
Timeout: &metav1.Duration{Duration: timeout},
Type: sourcev1.HelmRepositoryTypeOCI,
URL: fmt.Sprintf("oci://%s/testrepo", testRegistryServer.registryHost),
Timeout: &metav1.Duration{Duration: timeout},
Provider: sourcev1.GenericOCIProvider,
Type: sourcev1.HelmRepositoryTypeOCI,
},
}
obj := &sourcev1.HelmChart{
Expand Down
59 changes: 59 additions & 0 deletions controllers/helmrepository_controller_oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"net/url"
"os"
"strings"
"time"

helmgetter "helm.sh/helm/v3/pkg/getter"
Expand All @@ -41,10 +42,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/oci"
"github.com/fluxcd/pkg/oci/auth/login"
"github.com/fluxcd/pkg/runtime/conditions"
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
"github.com/google/go-containerregistry/pkg/name"

"github.com/fluxcd/source-controller/api/v1beta2"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
Expand Down Expand Up @@ -204,6 +208,9 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
// block at the very end to summarize the conditions to be in a consistent
// state.
func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta2.HelmRepository) (result ctrl.Result, retErr error) {
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()

oldObj := obj.DeepCopy()

defer func() {
Expand Down Expand Up @@ -296,6 +303,19 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
}
}

if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, obj)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
result, retErr = ctrl.Result{}, e
return
}
if auth != nil {
loginOpts = append(loginOpts, auth)
}
}

// Create registry client and login if needed.
registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
if err != nil {
Expand Down Expand Up @@ -366,3 +386,42 @@ func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime
}
r.Eventf(obj, eventType, reason, msg)
}

// oidcAuth generates the OIDC credential authenticator based on the specified cloud provider.
func oidcAuth(ctx context.Context, obj *sourcev1.HelmRepository) (helmreg.LoginOption, error) {
url := strings.TrimPrefix(obj.Spec.URL, sourcev1.OCIRepositoryPrefix)
ref, err := name.ParseReference(url)
if err != nil {
return nil, fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err)
}

loginOpt, err := loginWithManager(ctx, obj.Spec.Provider, url, ref)
if err != nil {
return nil, fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err)
}

return loginOpt, nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The code in this function combined with loginWithManager() below, it seems like registry.OIDCAdaptHelper() can just take the result of OCIRepositoryReconciler.oidcAuth(). The arguments of OCIRepositoryReconciler.oidcAuth() can just be the URL and the provider name to not make it source type specific. It should be okay to convert OCIRepositoryReconciler.oidcAuth() into an independent function oidcAuth().

In the future, I think we can make a private package with a lot of the OCI related code here and in OCIRepo reconciler.

Copy link
Member Author

@souleb souleb Aug 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

true, it would be better. But as you said OCIRepositoryReconciler.oidcAuth() is a receiver function, so I thought that this would need a specific PR to refactor it.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay to change it separately.


func loginWithManager(ctx context.Context, provider, url string, ref name.Reference) (helmreg.LoginOption, error) {
opts := login.ProviderOptions{}
switch provider {
case sourcev1.AmazonOCIProvider:
opts.AwsAutoLogin = true
case sourcev1.AzureOCIProvider:
opts.AzureAutoLogin = true
case sourcev1.GoogleOCIProvider:
opts.GcpAutoLogin = true
}

auth, err := login.NewManager().Login(ctx, url, ref, opts)
if err != nil {
return nil, err
}

if auth == nil {
return nil, nil
}

return registry.OIDCAdaptHelper(auth)
}
3 changes: 2 additions & 1 deletion controllers/helmrepository_controller_oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) {
SecretRef: &meta.LocalObjectReference{
Name: secret.Name,
},
Type: sourcev1.HelmRepositoryTypeOCI,
Provider: sourcev1.GenericOCIProvider,
Type: sourcev1.HelmRepositoryTypeOCI,
},
}
g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
Expand Down
36 changes: 34 additions & 2 deletions docs/api/source.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,9 @@ Kubernetes meta/v1.Duration
</td>
<td>
<em>(Optional)</em>
<p>Timeout of the index fetch operation, defaults to 60s.</p>
<p>Timeout is used for the index fetch operation for an HTTPS helm repository,
and for remote OCI Repository operations like pulling for an OCI helm repository.
Its default value is 60s.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -863,6 +865,20 @@ string
When this field is set to &ldquo;oci&rdquo;, the URL field value must be prefixed with &ldquo;oci://&rdquo;.</p>
</td>
</tr>
<tr>
<td>
<code>provider</code><br>
<em>
string
</em>
</td>
<td>
<em>(Optional)</em>
<p>Provider used for authentication, can be &lsquo;aws&rsquo;, &lsquo;azure&rsquo;, &lsquo;gcp&rsquo; or &lsquo;generic&rsquo;.
This field is optional, and only taken into account if the .spec.type field is set to &lsquo;oci&rsquo;.
When not specified, defaults to &lsquo;generic&rsquo;.</p>
</td>
</tr>
</table>
</td>
</tr>
Expand Down Expand Up @@ -2347,7 +2363,9 @@ Kubernetes meta/v1.Duration
</td>
<td>
<em>(Optional)</em>
<p>Timeout of the index fetch operation, defaults to 60s.</p>
<p>Timeout is used for the index fetch operation for an HTTPS helm repository,
and for remote OCI Repository operations like pulling for an OCI helm repository.
Its default value is 60s.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -2392,6 +2410,20 @@ string
When this field is set to &ldquo;oci&rdquo;, the URL field value must be prefixed with &ldquo;oci://&rdquo;.</p>
</td>
</tr>
<tr>
<td>
<code>provider</code><br>
<em>
string
</em>
</td>
<td>
<em>(Optional)</em>
<p>Provider used for authentication, can be &lsquo;aws&rsquo;, &lsquo;azure&rsquo;, &lsquo;gcp&rsquo; or &lsquo;generic&rsquo;.
This field is optional, and only taken into account if the .spec.type field is set to &lsquo;oci&rsquo;.
When not specified, defaults to &lsquo;generic&rsquo;.</p>
</td>
</tr>
</tbody>
</table>
</div>
Expand Down
Loading