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

Create default passwords when dev mode is set. #441 #442

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
3a974cb
WIP. Create default passwords when dev mode is set. #441
cmoulliard Nov 8, 2024
a9c683f
Set the devMode to BuildCustomization
cmoulliard Nov 8, 2024
6d85851
Pass the proper object containing devMode to the function populating …
cmoulliard Nov 8, 2024
8dedc1e
Adding the code to patch the argocd-secret to use the hashed password…
cmoulliard Nov 8, 2024
704bc78
Use cost 0 as argocd code #441
cmoulliard Nov 8, 2024
ee9dc06
Re-creating too the inital admin secret for argocd. #441
cmoulliard Nov 8, 2024
d392287
Move the current time function. Generate the missing field. #441
cmoulliard Nov 8, 2024
0c62564
Include needed argocd k8s resources and change the code to set the de…
cmoulliard Nov 12, 2024
1e1b980
Fix wrong key as account should be accounts
cmoulliard Nov 12, 2024
3d884da
Change the gitea user from giteaAdmin to developer for a user's devel…
cmoulliard Nov 12, 2024
f31a08e
Reverting to giteaAdmin till we know why a different user - developer…
cmoulliard Nov 12, 2024
c355ba1
Change the number from 58 to 59 as we install a new ConfigMap - RBAC …
cmoulliard Nov 12, 2024
8158551
Remove from the util go file the setupLog and replace it with fmt.Err…
cmoulliard Nov 21, 2024
78e1e40
Removed non used argocd constants
cmoulliard Nov 21, 2024
7d81b27
Use r.client instead of k8s.GetKubeClient()
cmoulliard Nov 21, 2024
a8c3016
Rename the flag from dev to dev-mode
cmoulliard Nov 21, 2024
80a785b
Use an unstructured object to avoid managing fields we do not care ab…
cmoulliard Nov 21, 2024
3c3511d
Include the developer username and password to the command get secret
cmoulliard Nov 22, 2024
bf8e2cc
Refactor the code to update the argocd password post reconciliation. …
cmoulliard Dec 9, 2024
f76db50
Refactor the code to update the gitea password post reconciliation. #441
cmoulliard Dec 10, 2024
86b26a2
Change the logging level from info to debug
cmoulliard Dec 11, 2024
35fc902
Rename devMode, dev-mode to staticPasswords and static-passwords. Fix…
cmoulliard Dec 11, 2024
e4c78de
Remove else block not needed when we verify if the password changed
cmoulliard Dec 11, 2024
700a841
Improve the code and remove the non needed return statement
cmoulliard Dec 11, 2024
5830b22
Improve code and variables
cmoulliard Dec 11, 2024
d1ed81a
Remove non used functions
cmoulliard Dec 11, 2024
f0754f7
Created a new util package for gitea and moved there: const and funct…
cmoulliard Dec 11, 2024
3ab4fd3
Move GiteaBaseUrl to the util package of gitea
cmoulliard Dec 11, 2024
c8ac946
Fix error r.r.GiteaBaseUrl => util.r.GiteaBaseUrl
cmoulliard Dec 11, 2024
e02a5d2
Refactor the code to also patch the secret's password of gitea and ar…
cmoulliard Dec 11, 2024
f67a91c
Regenerate the token for gitea
cmoulliard Dec 11, 2024
dbf56c2
Fix wrong call to the function: GetGiteaToken
cmoulliard Dec 11, 2024
d029f62
Reverting the change to use as user arg: dev-mode instead of staticPa…
cmoulliard Dec 12, 2024
570ca30
Define an util const: StaticPassword
cmoulliard Dec 13, 2024
2fe31a3
Move the function ArgocdInitialAdminSecretObject() to the argocd.go f…
cmoulliard Dec 13, 2024
e4831cd
Rename functions. Remove the var about status and change the signatur…
cmoulliard Dec 13, 2024
1cd1773
Remove devMode as not used
cmoulliard Dec 13, 2024
70e3d37
Rename dev-mode to dev-password
cmoulliard Dec 13, 2024
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
13 changes: 7 additions & 6 deletions api/v1alpha1/gitrepository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import (
)

const (
GitProviderGitea = "gitea"
GitProviderGitHub = "github"
GiteaAdminUserName = "giteaAdmin"
SourceTypeLocal = "local"
SourceTypeRemote = "remote"
SourceTypeEmbedded = "embedded"
GitProviderGitea = "gitea"
GitProviderGitHub = "github"
GiteaAdminUserName = "giteaAdmin"
GiteaDeveloperUserName = "developer"
SourceTypeLocal = "local"
SourceTypeRemote = "remote"
SourceTypeEmbedded = "embedded"
)

type GitRepositorySpec struct {
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/localbuild_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type BuildCustomizationSpec struct {
Port string `json:"port,omitempty"`
UsePathRouting bool `json:"usePathRouting,omitempty"`
SelfSignedCert string `json:"selfSignedCert,omitempty"`
DevMode bool `json:"devMode,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This field should be StaticPasswords. We need to make an effort to make what these fields do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 35fc902

}

type LocalbuildSpec struct {
Expand Down
1 change: 1 addition & 0 deletions hack/argo-cd/argocd-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: argocd-cm
data:
application.resourceTrackingMethod: annotation
accounts.developer: apiKey, login
timeout.reconciliation: 60s
resource.exclusions: |
- kinds:
Expand Down
12 changes: 12 additions & 0 deletions hack/argo-cd/argocd-rbac-dev.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app.kubernetes.io/name: argocd-rbac-cm
app.kubernetes.io/part-of: argocd
name: argocd-rbac-cm
namespace: argocd
data:
policy.csv: |
p, role:developer, applications, *, *, allow
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of having a separate account that has a very similar permissions as admin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an account for the developers and it only allows to handle applications

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you tell me if this is inline with what you are thinking?

  1. Use a random password for the developer and admin account if the dev password flag is unset.
  2. Use a static, known password for the developer and admin account if the dev password flag is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Option 2 => Use a known password for the developer and admin account if the dev password flag is set

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok that sounds good to me. Looks like Gitea static password isn't working for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok that sounds good to me. Looks like Gitea static password isn't working for some reason?

For gitea when using dev-mode, we should still use as user: giteaAdmin and password = developer

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are adding a user called developer to argocd, we may as well add the developer user in Gitea.

g, developer, role:developer
1 change: 1 addition & 0 deletions hack/argo-cd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- https://raw.githubusercontent.com/argoproj/argo-cd/v2.10.7/manifests/install.yaml
- argocd-rbac-dev.yaml

patches:
- path: dex-server.yaml
Expand Down
4 changes: 3 additions & 1 deletion pkg/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var (

type Build struct {
name string
devMode bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be staticPasswords

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 35fc902

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still set as devMode

cfg v1alpha1.BuildCustomizationSpec
kindConfigPath string
kubeConfigPath string
Expand Down Expand Up @@ -278,5 +279,6 @@ func isBuildCustomizationSpecEqual(s1, s2 v1alpha1.BuildCustomizationSpec) bool
s1.IngressHost == s2.IngressHost &&
s1.Port == s2.Port &&
s1.UsePathRouting == s2.UsePathRouting &&
s1.SelfSignedCert == s2.SelfSignedCert
s1.SelfSignedCert == s2.SelfSignedCert &&
s1.DevMode == s2.DevMode
}
4 changes: 4 additions & 0 deletions pkg/cmd/create/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
const (
recreateClusterUsage = "Delete cluster first if it already exists."
buildNameUsage = "Name for build (Prefix for kind cluster name, pod names, etc)."
devModeUsage = "When enabled, the platform will run the core packages with developer password."
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. I am just not sold on the name dev mode. idpbuilder is primary used for development and testing purposes already. I think we should use more explicit name with a short flag.
  2. Even if we accept dev mode, we shouldn't limit the flag name to just passwords. dev mode could mean a lot of different things. This should be a meta flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. idpbuilder is primary used for development and testing purposes already. I think we should use more explicit name with a short flag.

If this is the case, then we should install gitea and argocd using a non generated password ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2. Even if we accept dev mode, we shouldn't limit the flag name to just passwords. dev mode could mean a lot of different things. This should be a meta flag.

What about using the the parameter: --devPwd or --devPassword as boolean @nabuskey

Copy link
Collaborator

@nabuskey nabuskey Nov 21, 2024

Choose a reason for hiding this comment

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

I am ok with dev-pwd or dev-password since we use snake case for our flags. We can also have --dev-mode flag that is essentially a convenient flag that enables dev passwords and any other QOL stuff for dev purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally we aligned our paths => I will then use --dev-mode

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I meant to have two flags. (--dev-password OR --dev-pwd ) AND --dev-mode because we could enable other QOL features under --dev-mode other than static passwords.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be fair is the --dev- prefix needed? I would imagine we could just have a --static-creds parameter or --static-credentials to set the password statically rather then dynamically? Unless there's a reason why we feel --dev- prefix is valuable?

The only argument I could maybe gather is that since it is no longer than admin credentials and it is developer credentials that --dev- prefix gives clarity to that but I think a good description here could explain that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--dev-mode is a pretty common parameter used by many developer's tools and CLI which could also be used to support different additional features in the future. On the opposite --static-creds or --static-credentials parameters are probably too restrictive, don't help the users as they have no idea about what static word means

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I meant to have two flags. (--dev-password OR --dev-pwd ) AND --dev-mode because we could enable other QOL features under --dev-mode other than static passwords.

I still think this needs to be the case.

kubeVersionUsage = "Version of the kind kubernetes cluster to create."
extraPortsMappingUsage = "List of extra ports to expose on the docker container and kubernetes cluster as nodePort " +
"(e.g. \"22:32222,9090:39090,etc\")."
Expand All @@ -40,6 +41,7 @@ var (
// Flags
recreateCluster bool
buildName string
devMode bool
kubeVersion string
extraPortsMapping string
kindConfigPath string
Expand Down Expand Up @@ -67,6 +69,7 @@ func init() {
CreateCmd.PersistentFlags().StringVar(&buildName, "build-name", "localdev", buildNameUsage)
CreateCmd.PersistentFlags().MarkDeprecated("build-name", "use --name instead.")
CreateCmd.PersistentFlags().StringVar(&buildName, "name", "localdev", buildNameUsage)
CreateCmd.PersistentFlags().BoolVar(&devMode, "dev-mode", false, devModeUsage)
Copy link
Collaborator

Choose a reason for hiding this comment

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

add another flag here with the name dev-password.

Copy link
Contributor Author

@cmoulliard cmoulliard Dec 13, 2024

Choose a reason for hiding this comment

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

ok but then what will be the purpose of the flag dev-mode if now we have 2 flags: dev-mode and dev-password ? @nabuskey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed devMode from build.go as non used: 1cd1773

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CreateCmd.PersistentFlags().StringVar(&kubeVersion, "kube-version", "v1.30.3", kubeVersionUsage)
CreateCmd.PersistentFlags().StringVar(&extraPortsMapping, "extra-ports", "", extraPortsMappingUsage)
CreateCmd.PersistentFlags().StringVar(&kindConfigPath, "kind-config", "", kindConfigPathUsage)
Expand Down Expand Up @@ -143,6 +146,7 @@ func create(cmd *cobra.Command, args []string) error {
IngressHost: ingressHost,
Port: port,
UsePathRouting: pathRouting,
DevMode: devMode,
},

CustomPackageDirs: absDirPaths,
Expand Down
13 changes: 13 additions & 0 deletions pkg/cmd/get/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"os"
"path/filepath"
"strings"
"text/template"

"github.com/cnoe-io/idpbuilder/api/v1alpha1"
Expand Down Expand Up @@ -206,6 +207,18 @@ func secretToTemplateData(s v1.Secret) TemplateData {
for k, v := range s.Data {
data.Data[k] = string(v)
}

// TODO: The following code should be reviewed and improved as the secret containing the developer username/password is argocd-secret
// where the password has been bcrypted and by consequence we cannot get and decode it from the secret
// This is why we are going to add it here BUT it will be displayed every time no matter if --dev-mode has been used or not
if strings.Contains(s.Name, "gitea") {
data.Data["username-developer"] = "giteAdmin"
data.Data["password-developer"] = "developer"
} else if strings.Contains(s.Name, "argocd") {
data.Data["username-developer"] = "developer"
data.Data["password-developer"] = "developer"
}

Copy link
Collaborator

@nabuskey nabuskey Nov 22, 2024

Choose a reason for hiding this comment

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

We can get the localbuild object from K8s since it's essentially our way of storing configurations. You added the DevMode field so you can just check the value. We should change that field name btw. devPassword makes sense. There's a typo: giteAdmin -> giteaAdmin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can get the localbuild object from K8s since it's essentially our way of storing configurations.

Yes but the flag --dev-mode or --dev-password is not passed at all to the command: idp get secrets and users will not like to perform such a command idp get secrets --dev-mode.

return data
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/gitrepository/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ func (r *RepositoryReconciler) reconcileGitRepo(ctx context.Context, repo *v1alp
return ctrl.Result{}, fmt.Errorf("getting git provider credentials: %w", err)
}

if r.Config.DevMode {
creds.password = "developer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use the string developer in different places. We should define it somewhere and reference it from everywhere it's used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 570ca30

}

err = provider.setProviderCredentials(ctx, repo, creds)
if err != nil {
return ctrl.Result{}, fmt.Errorf("setting git provider credentials: %w", err)
Expand Down
1 change: 1 addition & 0 deletions pkg/controllers/gitrepository/gitea.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (g *giteaProvider) getProviderCredentials(ctx context.Context, repo *v1alph
if !ok {
return gitProviderCredentials{}, fmt.Errorf("%s key not found in secret %s in %s ns", giteaAdminPasswordKey, repo.Spec.SecretRef.Name, repo.Spec.SecretRef.Namespace)
}

return gitProviderCredentials{
username: string(username),
password: string(password),
Expand Down
28 changes: 27 additions & 1 deletion pkg/controllers/localbuild/argo.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package localbuild
import (
"context"
"embed"

"fmt"
"github.com/cnoe-io/idpbuilder/api/v1alpha1"
"github.com/cnoe-io/idpbuilder/globals"
"github.com/cnoe-io/idpbuilder/pkg/k8s"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -15,6 +17,13 @@ import (
//go:embed resources/argo/*
var installArgoFS embed.FS

const (
argocdDevModePassword = "developer"
argocdInitialAdminSecretName = "argocd-initial-admin-secret"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this to globals. We reference these in the get command too.
https://github.com/cnoe-io/idpbuilder/blob/main/globals/project.go

argocdNamespace = "argocd"
argocdIngressURL = "%s://argocd.cnoe.localtest.me:%s"
)

func RawArgocdInstallResources(templateData any, config v1alpha1.PackageCustomization, scheme *runtime.Scheme) ([][]byte, error) {
return k8s.BuildCustomizedManifests(config.FilePath, "resources/argo", installArgoFS, scheme, templateData)
}
Expand Down Expand Up @@ -57,3 +66,20 @@ func (r *LocalbuildReconciler) ReconcileArgo(ctx context.Context, req ctrl.Reque
resource.Status.ArgoCD.Available = true
return ctrl.Result{}, nil
}

func (r *LocalbuildReconciler) ArgocdInitialAdminSecretObject() corev1.Secret {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this needs to be exported. This is more like a utility function. Since we do something like this in the get command, we should move this to the util package.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be moved to utils.

return corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: argocdInitialAdminSecretName,
Namespace: argocdNamespace,
},
}
}

func (r *LocalbuildReconciler) ArgocdBaseUrl(config v1alpha1.BuildCustomizationSpec) string {
return fmt.Sprintf(argocdIngressURL, config.Protocol, config.Port)
}
4 changes: 2 additions & 2 deletions pkg/controllers/localbuild/argo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func TestGetK8sInstallResources(t *testing.T) {
t.Fatalf("GetK8sInstallResources() error: %v", err)
}

if len(objs) != 58 {
t.Fatalf("Expected 58 Argo Install Resources, got: %d", len(objs))
if len(objs) != 59 {
t.Fatalf("Expected 59 Argo Install Resources, got: %d", len(objs))
}
}

Expand Down
Loading
Loading